Jens-G opened a new pull request, #3981: URL: https://github.com/apache/thrift/pull/3981
`TPipeServer` did not implement `interruptChildren()`, so `TServer::stop()` could not end a client that was waiting on a pipe, and `serve()` did not return while a client stayed connected. See [THRIFT-3590](https://issues.apache.org/jira/browse/THRIFT-3590). This builds on the cancellation that #3878 (THRIFT-6329) added to `TPipe`. This change: - **Adds `TPipeServer::interruptChildren()`.** The server shares one manual-reset event with every pipe it accepts, through a new `TPipe(TAutoHandle&, std::shared_ptr<TManualResetEvent>, config)` constructor. `interruptChildren()` sets that event. - From then on, `read()` on one of those pipes throws `TTransportException::INTERRUPTED`, even when the client has sent data. That matches a `TSocket` that `TServerSocket` accepted, so a client that keeps sending cannot keep `stop()` from finishing either. - A `write()` that waits for the client is interrupted as well; `TSocket` does not do that. - **Interrupts the children when the server closes**, as `TServerSocket::close()` does. I checked on Linux that closing or destroying a `TServerSocket` interrupts its children's reads. - **Adds `setInterruptableChildren(bool)`**, with the same meaning and default as on `TServerSocket`. Off keeps the old behaviour. It must be called before `listen()`, otherwise it throws `std::logic_error`. On POSIX, `TPipeServer` is a `TServerSocket`, so portable code now behaves the same on both platforms. - **Leaves anonymous pipes alone.** They do synchronous I/O and are not interrupted. **Behaviour change (Breaking-Change on THRIFT-3590):** `TServer::stop()` now disconnects the clients of a `TPipeServer` instead of waiting for them, as it does for `TServerSocket` since THRIFT-2441. Closing the server interrupts its children too. `setInterruptableChildren(false)` restores the old behaviour. `lib/cpp/test/TPipeInterruptTest.cpp` mirrors the child cases of `TSocketInterruptTest`. The peek cases are left out, because `TPipe::peek()` does not wait. It also adds: - that an interrupted child reads no more, whether the client's data has just arrived or is already buffered in the child; - the scenario from the ticket: `stop()` with a silent client connected, for `TThreadedServer` and for `TSimpleServer`. Verified on Windows (GitHub-hosted runner, MSVC, Release). Each case ran 10 times, each run in its own process: | | `interruptChildren()` wakes a child read / write | an interrupted child ignores received / buffered data | closing the server wakes a child read | `stop()` ends `serve()` with a silent client (threaded / simple) | |---|---|---|---|---| | #3878 + new tests | fails 10/10: not woken up within 10 s | fails 10/10: the child still reads | fails 10/10: not woken up within 10 s | fails 10/10: `serve()` still running after 10 s | | this change | passes 10/10 | passes 10/10 | passes 10/10 | passes 10/10 | Two cases test the new API and cannot be built without this change: `setInterruptableChildren(false)` leaves a child alone, and the setting cannot be changed after `listen()`. Both pass 10/10 with the change. - The four THRIFT-6329 cases passed 10/10 in both runs. - The whole `TInterruptTest` passes with the change. - The runs are [#3878 + new tests](https://github.com/Jens-G/thrift/actions/runs/36265749222) and [this change](https://github.com/Jens-G/thrift/actions/runs/36265748937). They ran on a scaffolding branch that builds only `TInterruptTest`; the change under test is the same patch as this PR. - [x] Did you create an [Apache Jira](https://issues.apache.org/jira/projects/THRIFT/issues/) ticket? [THRIFT-3590](https://issues.apache.org/jira/browse/THRIFT-3590) - [x] If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"? - [x] Did you squash your changes to a single commit? - [x] Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change" 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
