Jens Geyer created THRIFT-6244:
----------------------------------

             Summary: TNonblockingServerTest intermittently crashes or hangs in 
bad_alloc_does_not_end_the_process
                 Key: THRIFT-6244
                 URL: https://issues.apache.org/jira/browse/THRIFT-6244
             Project: Thrift
          Issue Type: Bug
          Components: C++ - Library
            Reporter: Jens Geyer
             Fix For: 0.25.0


{{TNonblockingServerTest}} fails intermittently on AppVeyor, always in the case 
{{bad_alloc_does_not_end_the_process}}: either the test binary crashes (ctest 
reports SEGFAULT) or it hangs until the 300 s ctest timeout.

The case was added with 
[eda9f8d71|https://github.com/apache/thrift/commit/eda9f8d7171d88082ae5f00642411c11c92ba8a4],
 and the first AppVeyor build that contained it (0.25.0.10466) was also the 
first failure. Since then 11 of 229 jobs have failed (MSVC2022: 8 crashes and 2 
timeouts out of 192 jobs; MinGW: 1 timeout out of 37), and 11 of 39 builds went 
red because of it. A search of all failed AppVeyor job logs since 2025-07-22 
finds no earlier failure of this test.

Example jobs:
* crash: 
[0.25.0.10469|https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/54684673/job/fw4ai49opu5j66kw]
* timeout: 
[0.25.0.10466|https://ci.appveyor.com/project/ApacheSoftwareFoundation/thrift/builds/54684499/job/ueslnq02ob7c29w7]

h2. Cause 1: the fixture destroys the server while a thread-pool task is still 
running (crash)

In the crashing runs, the server output of this case ends like this (timestamps 
removed):
{noformat}
TNonblockingServer: IO thread #0 entering loop...
TNonblockingServer: IO thread #0 run() done!
TNonblocking: join done for IO thread #0
TNonblockingServer: process() exception: class std::bad_alloc: bad allocation
TNonblockingServer: failed to notifyIOThread, closing.
{noformat}
The worker handling the deliberately failed call reaches its catch handler only 
after the fixture has already stopped the server:
* {{FailsFirstCallProcessor}} sets its flag and throws. Meanwhile the second 
worker serves {{canCommunicate()}}, the test body returns, and the fixture is 
torn down.
* The fixture destructor calls {{stop()}} on the server and {{join()}} on the 
serve thread, but never stops the {{ThreadManager}}.
* The fixture members are then destroyed in reverse declaration order, the 
server before the thread manager. The {{TNonblockingServer}} destructor closes 
and deletes every connection and the IO threads, including the notification 
pipe; only afterwards does the {{ThreadManager}} destructor wait for its 
workers.
* The pool task holds a raw pointer to its {{TConnection}}. The task that is 
still unwinding then calls {{notifyIOThread()}} on the deleted connection and, 
when that fails, {{close()}}.

Four passing runs show the same late ordering, with {{run() done!}} logged 
before the {{bad_alloc}} line; they finished just in time.

h2. Cause 2: the injected failure is not tied to the call meant to receive it 
(timeout)

{{FailsFirstCallProcessor}} fails whichever call reaches {{process()}} first, 
tracked in a plain {{bool}} that both pool workers read and write. The call 
that consumes the failure gets no reply. If that is the {{addString}} call from 
{{canCommunicate()}}, its client, which has no receive timeout, blocks until 
ctest kills the test. The missing reply itself is the server behaviour tracked 
in THRIFT-6216; fixing that would turn the hang into a failed check, but would 
not remove the race. Two variants have been observed:
* Windows: both workers saw the flag unset, and the log shows the {{bad_alloc}} 
line twice.
* Linux, running the unmodified test under ASan with CPU load from 8 parallel 
loops: the order was inverted. The call meant to fail was served normally, its 
reply hitting the already closed connection ("Broken pipe"), and the call from 
{{canCommunicate()}} received the failure. 2 of 1200 runs hung; 400 sequential 
runs all passed. The crash did not reproduce on Linux.

h2. Proposed fix (test only)

Both parts are needed, because each one closes only one of the two symptoms:
# In the fixture destructor, stop the {{ThreadManager}} after joining the serve 
thread and before the server is destroyed. A task still in flight then 
completes while its connection and the notification pipe still exist. On its 
own this does not help with the timeout, because the test body hangs and 
teardown is never reached.
# Tie the injected failure to the call meant to receive it: for example, use a 
{{std::atomic<bool>}} and call {{canCommunicate()}} only after that call has 
consumed the failure, signalled for instance through a {{Monitor}} right before 
the throw. On its own this does not help with the crash, because the failing 
task can still run into teardown.

Optionally, a receive timeout on the socket in {{canCommunicate()}} would make 
a regression fail within seconds instead of after 300 s.

Out of scope: {{TNonblockingServer::stop()}} and the server destructor do not 
wait for tasks still running in the thread pool, so an application currently 
has to stop the server, join the serve thread and stop the {{ThreadManager}} 
before destroying the server. Whether the server itself should wait is a 
separate question.

_Drafted with AI assistance (Claude Opus 5)._



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to