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 inprocess oneshot #100
Merged
Conversation
Holding a global lock on the `ONE_SHOT_SERVERS` map while doing the blocking `accept()` invocation is a very bad idea -- aside from possible long-range concurrency problems or deadlocks, it actually creates a race condition between `accept()` and `connect()`, which the existing test case triggers pretty much every time on my system. The solution used here is not the most elegant IMHO -- but it's the only one I can think of that preserves the current abstraction, with `accept()` and `connect()` methods on `ServerRecord`. Possible more elegant (but also way more invasive) solutions would be moving the `conn_receiver` (along with the `accept()` implementation) to `OsIpcOneShotServer`; or even keeping *only* `conn_receiver` in `OsIpcOneShotServer`, while the actual data channel is only created and sent over the connect channel in the connect/accept phase -- thus bringing it more in line with the socket implementation...
|
@bors-servo: r=pcwalton
|
|
|
bors-servo
added a commit
that referenced
this pull request
Aug 31, 2016
|
|
With the deadlock in the `inprocess` implementation fixed, this should work fine on Windows now.
…ect()` Invoking `accept()` after `connect()` used to fail, because the server record was being dropped while doing the `connect()`. Now dropping it after the `accept()` instead -- which is safe for either invocation order, because `accept()` is blocking, and thus never finishes before `connect()` is done. Also adding a separate `server_connect_first` test case to explicitly test this situation. (And modifying the original test case to ensure it always tests the other situation.)
|
@bors-servo: r=emilio,pcwalton
|
|
|
bors-servo
added a commit
that referenced
this pull request
Aug 31, 2016
|
|
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.
emilio commentedAug 31, 2016
Rebase of #90.
@antrik sent a mail to the list saying he was not going to be active, so I took care of rebasing this.