Skip to content

Conversation

skenqbx
Copy link
Contributor

@skenqbx skenqbx commented May 22, 2015

This fixes a race condition introduced in 80342f6.

socket.destroy(err) only emits the passed error when socket._writableState.errorEmitted === false, ssl.onerror sets errorEmitted = true just before calling socket.destroy().

See #1119 & #1711

/cc @indutny

@skenqbx
Copy link
Contributor Author

skenqbx commented May 22, 2015

Maybe 80342f6 and this PR (IFF it lands) should also be backported to v1.x (#1736)

@indutny
Copy link
Member

indutny commented May 22, 2015

May I ask you to elaborate on commit log? Personally, I forgot about what was the reason for this change, and can't remember it now from looking at the commit message ;)

The PR has the text, so I guess this is what we actually may want to have in commit log as well.

@indutny
Copy link
Member

indutny commented May 22, 2015

Otherwise LGTM

This fixes a race condition introduced in 80342f6.
`socket.destroy(err)` only emits the passed error
when `socket._writableState.errorEmitted === false`,
`ssl.onerror` sets `errorEmitted = true`
just before calling `socket.destroy()`.

See #1119
See #1711
@skenqbx
Copy link
Contributor Author

skenqbx commented May 22, 2015

Commit log updated. Thank you for the prompt review!

indutny pushed a commit that referenced this pull request May 22, 2015
This fixes a race condition introduced in 80342f6.
`socket.destroy(err)` only emits the passed error when
`socket._writableState.errorEmitted === false`, `ssl.onerror`
sets `errorEmitted = true` just before calling
`socket.destroy()`.

See: #1119
See: #1711
PR-URL: #1769
Reviewed-By: Fedor Indutny <[email protected]>
@indutny
Copy link
Member

indutny commented May 22, 2015

Landed in 2a71f02, thank you!

@indutny indutny closed this May 22, 2015
@rvagg rvagg mentioned this pull request May 23, 2015
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
This fixes a race condition introduced in 80342f6.
`socket.destroy(err)` only emits the passed error when
`socket._writableState.errorEmitted === false`, `ssl.onerror`
sets `errorEmitted = true` just before calling
`socket.destroy()`.

See: nodejs/node#1119
See: nodejs/node#1711
PR-URL: nodejs/node#1769
Reviewed-By: Fedor Indutny <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants