test EOF behavior and fix missing EPOLLRDHUP events for TCP sockets#3574
test EOF behavior and fix missing EPOLLRDHUP events for TCP sockets#3574sporksmith merged 3 commits intoshadow:mainfrom
Conversation
392c42a to
1793164
Compare
cc02da7 to
ef4fae4
Compare
|
The epoll changes are definitely a bit cargo-culted. A critical eye from someone more familiar with this code is more than welcome :) |
On that note adding @robgjansen too; if you have time to look I think you might be the most familiar with this code? |
robgjansen
left a comment
There was a problem hiding this comment.
The epoll cargo-culting seems appropriately cargo-culted to me :) We should definitely have Steve review too though.
It was pleasant to see how much easier it was to add this to the Rust epoll module than the C module.
I couldn't convince myself why exactly we need support in both epoll modules... Do we? AFAIK, when the managed program creates an Epoll, we manage that state with the Rust mod, but when Shadow's syscall handler runs into a blocking syscall, we manage that wait task with the C mod. I guess a RDHUP might interact in both of the above paths?
Ultimately, the tests pass so I can't argue with that :D
Hmm, yeah I was and am a bit confused about the relationship between the two. There was definitely a bit of "keep adding to places where it looks like it should be until it works". I'll see if I can trace through a bit more; would definitely be preferable to not add dead code that might further confuse our future selves :) |
|
Cool! This looks like a difficult one to debug, so I'm glad you were able to narrow it down to this. |
I think the C epoll code is only used for the C |
While debugging https://gitlab.torproject.org/tpo/core/arti/-/issues/1972, it became apparent that the tokio runtime seems to rely on EPOLLRDHUP to know when a socket is closed, at least in some circumstances. This adds tests for this and several variations of EOF behavior. shadow fails the tests for socket file descriptors with EPOLLRDHUP.
0b2b351 to
5b2fd19
Compare
This reverts commit 5f6bd6c. This workaround is no longer needed since the underlying bugs have been fixed in shadow (See shadow/shadow#3100 and shadow/shadow#3574), and the CI has been bumped to use a version of shadow with these fixes.
While debugging
https://gitlab.torproject.org/tpo/core/arti/-/issues/1972, it became apparent that the tokio runtime seems to rely on EPOLLRDHUP to know when a socket is closed, at least in some circumstances.
This adds tests for this and several variations of EOF behavior.
A second commit adds EPOLLRDHUP support for TCP sockets and enables running the corresponding tests under shadow.