dominikh
commented
6 years ago I may have been quite wrong about my analysis, rendering this issue unactionable.
The server is very well allowed to respond to requests in an arbitrary order, and no order between requests exists once they hit the server. The onus seems to squarely be on the clients to not issue concurrent requests that may race. Indeed, with multiplexing, concurrency is often needed to achieve acceptable performance.
This leaves us with only one remaining issue, namely documenting the fact that go-p9p serves requests concurrently, but that can be its own issue.
justincormack
In its current implementation, go-p9p processes each incoming request in its own goroutine, and doesn't wait for their completion before processing subsequent requests.
This breaks the assumption in 9p that requests are processed in order, and that if request B happens after request A, B will observe the effects of A. For example, clunking a file makes it valid to reuse the fid; however if these two operations happened in the reverse order, it'd be a protocol error. Since go-p9p just fires-and-forgets goroutines for requests, no order exists.
9p clients are permitted to send multiple requests, but these are expected to be linearized and executed in order, and the bug in go-p9p is only evident with clients that do this, and under high loads. As far as I am aware, the Linux's 9pnet driver doesn't send multiple outstanding requests, but 9pfuse in plan9ports does.
While there is room for some concurrency, it isn't as trivially implemented as the faulty approach. One could decouple request parsing from request execution (c.f. Instruction Fetch and Instruction Decode in CPUs), or employ dependency analysis to execute requests that are independent of each other concurrently.
Finally, the presence of concurrency was surprising, as it isn't clearly documented. People may not expect methods on a Session to be executed concurrently.