tls: fix typo handle._reading => handle.reading
#994
Conversation
The problem does not manifest itself on unixes, because `uv_read_start()` always return 0 there. However on Windows on a second call `uv_read_start()` returns `UV__EALREADY` destroying all sockets on a read attempt. Set `.reading` property that is already handled by `net.js` code. Fix: nodejs#988
|
How fast can we push a patch release with this in it. It sounds like a nasty bug. Also, can we get a test in for this? |
|
@mikeal there are lots of failing tests on windows :) I think this should be enough. |
|
cc @iojs/crypto @rvagg |
|
Guys please land it if it LGTY, I'm going to sleep... |
even more concerning :( |
|
Jenkins has been doing some crazy stuff with git, the release was delayed because the 64-bit windows build machine kept on trying to build on a totally different commit than the others, ignoring the tag I gave it! My theory here is that the CI runs before release were using the wrong commit for testing on Windows machines giving us false-positives. |
|
The above assumption about CI was wrong, it was actually my fault, not Jenkins, full details in #995. |
| @@ -281,7 +281,7 @@ TLSSocket.prototype._wrapHandle = function(handle) { | |||
| tls.createSecureContext(); | |||
| res = tls_wrap.wrap(handle, context.context, options.isServer); | |||
| res._parent = handle; | |||
| res._reading = handle._reading; | |||
| res.reading = handle.reading; | |||
There was a problem hiding this comment.
Is it going to matter that handle.reading is undefined for non-net.Socket handles?
There was a problem hiding this comment.
@chrisdickinson can you think of a way to test this? I'd really like to get a release out in the next hour or so with this PR merged.
There was a problem hiding this comment.
On further reading, I think it should be harmless if it's not there.
|
LGTM. Going to merge it. |
|
https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/211/ good to go, lots of blue |
The problem does not manifest itself on unixes, because `uv_read_start()` always return 0 there. However on Windows on a second call `uv_read_start()` returns `UV__EALREADY` destroying all sockets on a read attempt. Set `.reading` property that is already handled by `net.js` code. Fix: #988 PR-URL: #994 Reviewed-By: Chris Dickinson <[email protected]>
|
Merged in 7554612. |
Notable changes: * tls: A typo introduced in the TLSWrap changes in #840 only encountered as a bug on Windows was not caught by the io.js CI system due to problems with the Windows build script and the Windows CI slave configuration, see Fixed in #994 & #1004 (Fedor Indutny) * npm: Upgrade npm to 2.6.1. See https://github.com/npm/npm/blob/master/CHANGELOG.md#v260-2015-02-12 for details. * Add new collaborators: - Robert Kowalski (@robertkowalski) - Christian Vaagland Tellnes (@tellnes) - Brian White (@mscdex)
The problem does not manifest itself on unixes, because
uv_read_start()always return 0 there. However on Windows on a secondcall
uv_read_start()returnsUV__EALREADYdestroying all sockets ona read attempt.
Set
.readingproperty that is already handled bynet.jscode.Fix: #988