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

osx,stream: retry sending handle on EMSGSIZE error #1739

Closed
wants to merge 2 commits into from

Conversation

santigimeno
Copy link
Member

On OSX when sending handles via sendmsg() it can return EMSGSIZE if
there isn't room in the socket output buffer to store the whole message.
In that's the case, return control to the loop and try again in the next
iteration.

Fixes: nodejs/node#14828

@santigimeno
Copy link
Member Author

I found out about this solution here though I understand this be problematic as it can lead to a busy loop.

@santigimeno
Copy link
Member Author

@@ -859,7 +859,16 @@ static void uv__write(uv_stream_t* stream) {
}

if (n < 0) {
if (errno != EAGAIN && errno != EWOULDBLOCK && errno != ENOBUFS) {
if (errno != EAGAIN && errno != EWOULDBLOCK && errno != ENOBUFS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation looks off here.

@@ -859,7 +859,16 @@ static void uv__write(uv_stream_t* stream) {
}

if (n < 0) {
if (errno != EAGAIN && errno != EWOULDBLOCK && errno != ENOBUFS) {
if (errno != EAGAIN && errno != EWOULDBLOCK && errno != ENOBUFS
#if defined(__APPLE__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how other feel about this, but I'm not a huge fan of have an ifdef in the middle of a statement like this.

On OSX when sending handles via `sendmsg()` it can return `EMSGSIZE` if
there isn't room in the socket output buffer to store the whole message.
In that's the case, return control to the loop and try again in the next
iteration.

Fixes: nodejs/node#14828
@santigimeno
Copy link
Member Author

PR updated to use a macro. @cjihrig what do you think?

@cjihrig
Copy link
Contributor

cjihrig commented Feb 19, 2018

I'm good with the macro.

@santigimeno
Copy link
Member Author

cjihrig pushed a commit that referenced this pull request Feb 21, 2018
On OSX when sending handles via `sendmsg()` it can return `EMSGSIZE` if
there isn't room in the socket output buffer to store the whole message.
If that's the case, return control to the loop and try again in the next
iteration.

Fixes: nodejs/node#14828
PR-URL: #1739
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig
Copy link
Contributor

cjihrig commented Feb 21, 2018

Landed in e6168df. Thanks!

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