Skip to content

Subtle bugs around closure handling in _unix_pipes.py and _windows_pipes.py #661

@njsmith

Description

@njsmith

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:

  1. task A calls receive_some
  2. the pipe isn't ready, so task A blocks in wait_readable
  3. some data arrives on the pipe, so task A gets rescheduled
  4. while task A is sitting on the run queue, task B closes the pipe
  5. task A gets scheduled, and calls os.read on 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions