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 upFix bugs in handling of socket addresses #52
Merged
+4
−4
Conversation
test.rs
Outdated
| tx2.send(person.clone()).unwrap(); | ||
| libc::exit(0); | ||
| }, | ||
| pid => pid, |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
antrik
Mar 18, 2016
Author
Contributor
Hm... Makes sense I guess. Where shall I put it though? It's used both in tests.rs and in platform/tests.rs...
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
antrik
Mar 18, 2016
Author
Contributor
Eh, actually I meant test.rs and platform/test.rs. So when you say tests.rs, do you actually mean test.rs as well -- or do you mean I should create a separate file for that?...
platform/linux/mod.rs
Outdated
| @@ -459,7 +459,7 @@ impl UnixOneShotServer { | |||
| loop { | |||
| let path_string = CString::new(b"/tmp/rust-ipc-socket.XXXXXX" as &[u8]).unwrap(); | |||
| path = path_string.as_bytes().iter().cloned().collect(); | |||
| if mktemp(path.as_mut_ptr() as *mut c_char) == ptr::null_mut() { | |||
| if *mktemp(path.as_mut_ptr() as *mut c_char) == 0 { | |||
This comment has been minimized.
This comment has been minimized.
The man page says that on error the name is set to an empty string. That means it's "", i.e. a pointer to '\0' -- *not* a NULL pointer! This prevented errors in mktemp() from being detected; resulting in other failures later on, when trying to use the empty string as a socket address...
CString::as_bytes() yields a slice that does *not* include the terminating '\0' byte; while the surrounding code subsequently copied the (unterminated) sequence, and passed the result to various libc function expecting a terminated string. This is a serious bug, causing random failure depending on unrelated memory contents following the copied vector. It triggered more or less frequent intermittent failures in the test suite (frequency depending on various other circumstances randomly causing changes in memory layout/contents); and it seems very likely that similar intermittent failures were triggered in actual application usage.
Never fill the last byte of the buffer, so the string always stays terminated. While not all implementation strictly need that, the man page strongly recommends it. This shouldn't actually make a difference for our present use case, as the (constant) string we are using is much shorter anyways. Still, better to have it correct -- who knows where this code ends up down the line...
|
I dropped the tests change from this PR, so it doesn't hold up the important fix. I'll submit it separately. |
|
@bors-servo: r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 18, 2016
Fix bugs in handling of socket addresses The main commit (the third one, about obtaining '\0'-terminated C strings) addresses a pretty serious bug, which is quite likely causing some intermittent failures. The others are minor. This also includes a commit with general improvement to some existing test cases, which is not strictly related to the issue at hand, but helped debugging it.
|
|
antrik
added a commit
to antrik/servo
that referenced
this pull request
Mar 19, 2016
This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux.
bors-servo
added a commit
to servo/servo
that referenced
this pull request
Mar 20, 2016
Update ipc-channel for another intermittent bug fix This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux. <!-- 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/10092) <!-- Reviewable:end -->
tyagiarpit
added a commit
to tyagiarpit/servo
that referenced
this pull request
Mar 23, 2016
This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux.
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 1, 2019
… fix (from antrik:update-ipc_channel-4); r=KiChjang This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux. Source-Repo: https://github.com/servo/servo Source-Revision: bcf077c53dcb836692fe52b7edb9bb14a80ff63b UltraBlame original commit: 12d2b65be1fca4a5de042a6775d8526ef2525606
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 1, 2019
… fix (from antrik:update-ipc_channel-4); r=KiChjang This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux. Source-Repo: https://github.com/servo/servo Source-Revision: bcf077c53dcb836692fe52b7edb9bb14a80ff63b UltraBlame original commit: 12d2b65be1fca4a5de042a6775d8526ef2525606
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 1, 2019
… fix (from antrik:update-ipc_channel-4); r=KiChjang This pulls in servo/ipc-channel#52 , and especially 8e2357604f7af8869b489b9682a2cf8b58177637, which fixes another likely cause of intermittent failures on GNU/Linux. Source-Repo: https://github.com/servo/servo Source-Revision: bcf077c53dcb836692fe52b7edb9bb14a80ff63b UltraBlame original commit: 12d2b65be1fca4a5de042a6775d8526ef2525606
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 commentedMar 18, 2016
The main commit (the third one, about obtaining '\0'-terminated C strings) addresses a pretty serious bug, which is quite likely causing some intermittent failures. The others are minor.
This also includes a commit with general improvement to some existing test cases, which is not strictly related to the issue at hand, but helped debugging it.