Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: emit errors happening before handshake's finish #1769

Closed
wants to merge 1 commit into from
Closed

tls: emit errors happening before handshake's finish #1769

wants to merge 1 commit into from

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 <fedor@indutny.com>
@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 <fedor@indutny.com>
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