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

Handle ENOBUFS by dynamically reducing packet size #62

Merged
merged 2 commits into from Apr 6, 2016

Conversation

@antrik
Copy link
Contributor

antrik commented Apr 3, 2016

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 2 commits Apr 3, 2016
The kernel sometimes fails to allocate a buffer large enough to send a
message even if it doesn't exceed the message size limit.
(Apparently when kernel memory is heavily fragmented during prolonged
memory pressure.)

Handle this by retrying / continuing the send with smaller packets when
we get the error.

(This is temporary: the next message will start with the full size
again. We assume that the kernel will recover sooner or later.)

Unfortunately there is no dedicated test case for this, as trying to
automate creating just the right conditions for triggering ENOBUFS
doesn't seem feasible. (Among other things, it can have nasty side
effects...)

However, the problem can be triggered by running the ordinary test suite
after "manually" creating memory pressure.
This reverts #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.
@eddyb
Copy link
Contributor

eddyb commented Apr 3, 2016

Works for me, both with and without browser.html. cc @Manishearth @pcwalton

@Manishearth
Copy link
Member

Manishearth commented Apr 4, 2016

Same

@Manishearth
Copy link
Member

Manishearth commented Apr 6, 2016

@pcwalton
Copy link
Collaborator

pcwalton commented Apr 6, 2016

@bors-servo: r+

Holy cow, Linux can be terrible sometimes.

@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

📌 Commit 782bbf0 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

Testing commit 782bbf0 with merge 828e68f...

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
@bors-servo
Copy link
Contributor

bors-servo commented Apr 6, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 782bbf0 into servo:master Apr 6, 2016
2 of 3 checks passed
2 of 3 checks passed
homu Testing commit 782bbf0 with merge 828e68f...
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
antrik added a commit to antrik/servo that referenced this pull request Apr 6, 2016
This fixes servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: ipc-channel was updated
to `uuid` 0.2 (don't know why...), while other crates are still with
0.1. This was blocking the urgent bug fix; and according to discussion
on #servo, the override should be OK in this case.
antrik added a commit to antrik/servo that referenced this pull request Apr 6, 2016
This fixes servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: ipc-channel was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with @mbrubeck on
IRC, the override should be OK in this case.
antrik added a commit to antrik/servo that referenced this pull request Apr 6, 2016
This fixes servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: ipc-channel was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with @mbrubeck on
IRC, the override should be OK in this case.
antrik added a commit to antrik/servo that referenced this pull request Apr 6, 2016
This fixes servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: `ipc-channel` was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with @mbrubeck on
IRC, the override should be OK in this case.
bors-servo added a commit to servo/servo that referenced this pull request Apr 6, 2016
Update ipc-channel for two important bug fixes

This fixes #10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: `ipc-channel` was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with @mbrubeck on
IRC, the override should be OK in this case.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10447)
<!-- Reviewable:end -->
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
…(from antrik:update-ipc_channel-5); r=jdm

This fixes servo/servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: `ipc-channel` was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with mbrubeck on
IRC, the override should be OK in this case.

Source-Repo: https://github.com/servo/servo
Source-Revision: 0b951f65b969ccc3445079a70686cf2146e365d7

UltraBlame original commit: c026ec733ec28813f9d6a5a4c7ab43d8cb8b3cac
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…(from antrik:update-ipc_channel-5); r=jdm

This fixes servo/servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: `ipc-channel` was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with mbrubeck on
IRC, the override should be OK in this case.

Source-Repo: https://github.com/servo/servo
Source-Revision: 0b951f65b969ccc3445079a70686cf2146e365d7

UltraBlame original commit: c026ec733ec28813f9d6a5a4c7ab43d8cb8b3cac
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
…(from antrik:update-ipc_channel-5); r=jdm

This fixes servo/servo#10260 by pulling in
servo/ipc-channel#61 (fix receive for messages
close to packet size) and servo/ipc-channel#62
(properly handle ENOBUFS); where the latter is not critical per se, as
there was a workaround already -- but that workaround aggrevated the
first bug, resulting in the urgent issue...

This bump requires a tidy override for `uuid`: `ipc-channel` was updated
to `uuid 0.2` in servo/ipc-channel#63 (don't
know why...), while other crates are still with `0.1`. That was blocking
this urgent bug fix; and according to a discussion with mbrubeck on
IRC, the override should be OK in this case.

Source-Repo: https://github.com/servo/servo
Source-Revision: 0b951f65b969ccc3445079a70686cf2146e365d7

UltraBlame original commit: c026ec733ec28813f9d6a5a4c7ab43d8cb8b3cac
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.