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

Thread Safe IO #8631

Closed
wants to merge 6 commits into from
Closed

Thread Safe IO #8631

wants to merge 6 commits into from

Conversation

anasazi
Copy link
Contributor

@anasazi anasazi commented Aug 20, 2013

libuv handles are tied to the event loop that created them. In order to perform IO, the handle must be on the thread with its home event loop. Thus, when as task wants to do IO it must first go to the IO handle's home event loop and pin itself to the corresponding scheduler while the IO action is in flight. Once the IO action completes, the task is unpinned and either returns to its home scheduler if it is a pinned task, or otherwise stays on the current scheduler.

Making new blocking IO implementations (i.e. files) thread safe is rather simple. Add a home field to the IO handle's struct in uvio and implement the HomingIO trait. Wrap every IO call in the HomingIO.home_for_io method, which will take care of the scheduling.

I'm not sure if this remains thread safe in the presence of asynchronous IO at the libuv level. If we decide to do that, then this set up should be revisited.

libuv does not always catch SIGPIPE.
Each IO handle has a home event loop, which created it.
When a task wants to use an IO handle, it must first make sure it is on that home event loop.
It uses the scheduler handle in the IO handle to send itself there before starting the IO action.
Once the IO action completes, the task restores its previous home state.
If it is an AnySched task, then it will be executed on the new scheduler.
If it has a normal home, then it will return there before executing any more code after the IO action.
@anasazi
Copy link
Contributor Author

anasazi commented Aug 20, 2013

@brson r?

@brson
Copy link
Contributor

brson commented Aug 20, 2013

@anasazi Can you convert relevant I/O tests to use run_in_mt_newsched_task from run_in_newsched_task so that we start getting more multithreaded I/O coverage?

@brson
Copy link
Contributor

brson commented Aug 20, 2013

There are some non-deterministic deadlocks when tests are converted so I've removed the r+ to give you time to investigate.

@brson
Copy link
Contributor

brson commented Aug 20, 2013

The deadlocks are specifically caused by the way the networking test cases are structured - they depend on scheduling behavior to ensure that the server sets up before the client.

bors added a commit that referenced this pull request Aug 21, 2013
libuv handles are tied to the event loop that created them. In order to perform IO, the handle must be on the thread with its home event loop. Thus, when as task wants to do IO it must first go to the IO handle's home event loop and pin itself to the corresponding scheduler while the IO action is in flight. Once the IO action completes, the task is unpinned and either returns to its home scheduler if it is a pinned task, or otherwise stays on the current scheduler.

Making new blocking IO implementations (i.e. files) thread safe is rather simple. Add a home field to the IO handle's struct in uvio and implement the HomingIO trait. Wrap every IO call in the HomingIO.home_for_io method, which will take care of the scheduling.

I'm not sure if this remains thread safe in the presence of asynchronous IO at the libuv level. If we decide to do that, then this set up should be revisited.
@bors bors closed this Aug 21, 2013
@anasazi anasazi mentioned this pull request Aug 21, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 21, 2022
Remove overlap between `manual_split_once` and `needless_splitn`

changelog: Remove overlap between [`manual_split_once`] and [`needless_splitn`]. Fixes some incorrect `rsplitn` suggestions for [`manual_split_once`]

Things that can trigger `needless_splitn` no longer trigger `manual_split_once`, e.g.

```rust
s.[r]splitn(2, '=').next();
s.[r]splitn(2, '=').nth(0);
s.[r]splitn(3, '=').next_tuple();
```

Fixes some suggestions:

```rust
let s = "should not match";

s.rsplitn(2, '.').nth(1);
// old -> Some("should not match")
Some(s.rsplit_once('.').map_or(s, |x| x.0));
// new -> None
s.rsplit_once('.').map(|x| x.0);

s.rsplitn(2, '.').nth(1)?;
// old -> "should not match"
s.rsplit_once('.').map_or(s, |x| x.0);
// new -> early returns
s.rsplit_once('.')?.0;
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants