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

Limit Linux sockets to 32kB buffers. #57

Merged
merged 2 commits into from Mar 28, 2016
Merged

Limit Linux sockets to 32kB buffers. #57

merged 2 commits into from Mar 28, 2016

Conversation

@eddyb
Copy link
Contributor

eddyb commented Mar 28, 2016

Based on the code in gonk/libui, this appears to fix servo/servo#9555.

let (a, b) = (results[0], results[1]);

// Limit the buffers to 32kB.
let buf_sz: c_int = 32 * 1024;

This comment has been minimized.

@pcwalton

pcwalton Mar 28, 2016

Collaborator

Can you factor this out into a top-level constant?

@pcwalton
Copy link
Collaborator

pcwalton commented Mar 28, 2016

@bors-servo: r+

It's really annoying that this is necessary, but…

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

📌 Commit 46151ee has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

Testing commit 46151ee with merge 0774ccb...

bors-servo added a commit that referenced this pull request Mar 28, 2016
Limit Linux sockets to 32kB buffers.

Based on [the code in `gonk/libui`](https://dxr.mozilla.org/mozilla-central/rev/63be002b4a803df1122823841ef7633b7561d873/widget/gonk/libui/InputTransport.cpp#130-134), this appears to fix servo/servo#9555.
@bors-servo
Copy link
Contributor

bors-servo commented Mar 28, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 46151ee into servo:master Mar 28, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@eddyb eddyb deleted the eddyb:patch-1 branch Mar 28, 2016
antrik added a commit to antrik/ipc-channel that referenced this pull request Apr 2, 2016
Just because we allocate a generous receive buffer for auxilllary data,
doesn't mean that a message can't use the space for other things when it
doesn't actually carry that amount of auxillary data.

In fact, there is no reason to expect every message we receive to have
auxillary data at all -- so let's just allocate a data buffer matching
the total maximal receive size. This will enable some optimisations to
be implemented; and it simplifies the code too.

Note that the extra space is not used (yet) when the sender does
explicit fragment size calculations; however, the sender presently tries
to send the message in a single packet first, without any advace size
checks at all -- so when the data size was close to the maximum packet
size, this actually resulted in the message not fitting in the receive
buffer!

This is what caused servo/servo#10260 , which
became (more) visible in Servo as a result of
servo#57 -- though the underlying
issue existed before...
antrik added a commit to antrik/ipc-channel that referenced this pull request Apr 3, 2016
This reverts servo#57

Now that we have handling for ENOBUFS in place, there is no need to work
around the problem by using a fixed small send size up front.
bors-servo added a commit that referenced this pull request Apr 6, 2016
Handle ENOBUFS by dynamically reducing packet size

This is a better fix for servo/servo#9555 : handling the problem when it actually occurs, without affecting performance otherwise.

It obsoletes (and removes) the workaround in #57
antrik added a commit to antrik/ipc-channel that referenced this pull request Apr 6, 2016
Just because we allocate a generous receive buffer for auxilllary data,
doesn't mean that a message can't use the space for other things when it
doesn't actually carry that amount of auxillary data.

In fact, there is no reason to expect every message we receive to have
auxillary data at all -- so let's just allocate a data buffer matching
the total maximal receive size. This will enable some optimisations to
be implemented; and it simplifies the code too.

Note that the extra space is not used (yet) when the sender does
explicit fragment size calculations; however, the sender presently tries
to send the message in a single packet first, without any advace size
checks at all -- so when the data size was close to the maximum packet
size, this actually resulted in the message not fitting in the receive
buffer!

This is what caused servo/servo#10260 , which
became (more) visible in Servo as a result of
servo#57 -- though the underlying
issue existed before...
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.

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