Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upLinux: Fix receive for message sizes close to packet size #61
Conversation
|
r? @pcwalton |
|
Btw, not sure if this is caused by this PR |
|
@Manishearth nah, that warning has been there for several months. I guess it comes from newer compiler versions? |
|
Perhaps. |
|
r? @pcwalton |
|
|
Introduce a new separate method for checking the maximum packet size that can be sent over the socket. This is mostly for the benefit of upcoming test cases, that will need to know this value -- which is why we make it public.
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 #57 -- though the underlying issue existed before...
|
Rebased. |
|
@bors-servo: r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 6, 2016
Linux: Fix receive for message sizes close to packet size Fixes servo/servo#10260 This supersedes #60 -- which is basically the same change; but now we have a test case, and an improved commit message. (Pointing out the bug fix; and also fixing a very confusing typo in the title...) The extra commit does some refactoring necessary for the new test 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 (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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
antrik commentedApr 2, 2016
Fixes servo/servo#10260
This supersedes #60 -- which is basically the same change; but now we have a test case, and an improved commit message. (Pointing out the bug fix; and also fixing a very confusing typo in the title...)
The extra commit does some refactoring necessary for the new test case.