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

Don't set SO_RCVBUF on UNIX sockets. #59

Closed
wants to merge 1 commit into from
Closed

Conversation

@eddyb
Copy link
Contributor

eddyb commented Apr 1, 2016

Fixes servo/servo#10260, although this might not be even a reasonable approach.
At this point it's mostly guesswork and Servo needs a lot more testing on linux.

@Manishearth
Copy link
Member

Manishearth commented Apr 1, 2016

@pcwalton
Copy link
Collaborator

pcwalton commented Apr 1, 2016

Bluh?

I mean, r=me I guess, but this is kinda scary…

@samlh
Copy link

samlh commented Apr 1, 2016

Pure speculation: if there are multiple senders, then perhaps the receive buffer needs to be large enough to receive from all simultaneously?

@antrik
Copy link
Contributor

antrik commented Apr 1, 2016

That's more than scary. For one, this will be very inefficient AIUI, as the receiver will always allocate a large receive buffer (208 KiB on x64 systems), but only ever use a small part of it...

More importantly, I don't see how this could really fix anything -- more likely it only papers over some other problem. In fact I believe I even know what exactly. I have some changes for that in the pipeline -- though they were meant only as (minor) optimisation and code simplification; I didn't realise until now that it's actually likely to cause bugs...

Investigating that will have to wait a day or two though.

@eddyb
Copy link
Contributor Author

eddyb commented Apr 1, 2016

Closed in favor of #60.

@eddyb eddyb closed this Apr 1, 2016
@eddyb eddyb deleted the eddyb:patch-1 branch Apr 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.