-
-
Notifications
You must be signed in to change notification settings - Fork 379
Description
This is something I just realized while thinking about a random question someone asked in #twisted:
In unix_pipes.py, suppose that the following sequence of events happens:
- task A calls
receive_some - the pipe isn't ready, so task A blocks in
wait_readable - some data arrives on the pipe, so task A gets rescheduled
- while task A is sitting on the run queue, task B closes the pipe
- task A gets scheduled, and calls
os.readon the closed fd, which if you're lucky raises an exception (EBADF), or if you're unlucky then a new fd got opened and assigned this value in between steps (4) and (5), and we end up reading from this random fd, which probably corrupts the state of some random other connection.
send_all has analogous issues.
I guess we need to re-check self._closed every time we use the fd, not just once on entry to the function.
This same issue can't happen with SocketStream, because when a socket is closed then it sets the underlying fd to -1, and so step 5 would call recv(-1, ...), which is always an EBADF, and the SocketStream code knows to convert EBADF into ClosedResourceError.
With trio.socket, the -1 thing means you can't get a wild read from a random fd, but there's no explicit handling of this case, so you will get an OSError(EBADF) instead of a proper ClosedResourceError. I guess that would probably be good to fix, though it's less urgent than the unix_pipes.py thing.