http://bugs.winehq.org/show_bug.cgi?id=60245 Bug ID: 60245 Summary: Threadpool worker retirement cancels pending overlapped I/O after callback returns Product: Wine Version: unspecified Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs@list.winehq.org Reporter: megadjc@gmail.com Target Milestone: --- Distribution: --- Created attachment 81952 --> http://bugs.winehq.org/attachment.cgi?id=81952 Two-patch series implementing ThreadIsIoPending and preventing threadpool worker retirement while thread-owned asynchronous I/O is pending. Wine can cancel ordinary overlapped I/O issued by a threadpool callback when the callback returns and the worker later becomes idle. I encountered this with DxO PureRAW 4, where a Qt threadpool callback issues an overlapped ReadFile() on a named pipe and returns while the read remains pending. Wine subsequently retires that worker after its threadpool worker timeout, which terminates the issuing thread and cancels the outstanding read. This can also be reproduced with a standalone Windows test and does not depend on PureRAW or Qt. Observed Wine behavior ====================== The relevant sequence is: threadpool callback -> ReadFile(named pipe, ..., OVERLAPPED) -> ReadFile returns FALSE / ERROR_IO_PENDING -> callback returns -> worker becomes idle -> Wine worker retirement timeout expires -> worker thread terminates -> pending overlapped read is cancelled -> GetOverlappedResult() reports ERROR_OPERATION_ABORTED With Wine's current: #define THREADPOOL_WORKER_TIMEOUT 5000 the PureRAW failure occurs reproducibly at approximately 5.05 seconds after the worker becomes idle. As an initial diagnostic experiment, changing only that timeout from 5000 ms to 60000 ms caused PureRAW to work correctly. This is not intended as the fix; it demonstrated that worker retirement was causing the I/O cancellation. Native Windows behavior ======================= I tested the corresponding behavior on Windows 11 with standalone programs. First, ordinary thread termination behaves as Wine currently expects: * A thread issues an ordinary overlapped named-pipe ReadFile(). * ReadFile returns ERROR_IO_PENDING. * The issuing thread exits. * The operation is cancelled. * GetOverlappedResult() returns ERROR_OPERATION_ABORTED (995). So Wine's general behavior of cancelling thread-owned asynchronous I/O when an ordinary thread terminates appears correct. Threadpool workers behave differently. Using a custom Windows threadpool with several workers: * one worker issued an ordinary overlapped named-pipe read and returned from its callback while the read remained pending; * the other workers owned no pending I/O; * the pool minimum was subsequently lowered so the workers became eligible for retirement. On native Windows, the workers with no I/O were retired, while the exact worker owning the pending overlapped read remained alive. In my native test the no-I/O workers were still alive at 65 seconds and had terminated by the 66-second sample. This is only an observed boundary and is not intended to imply that Windows has an internal 65-second timeout. The I/O-owning worker remained alive beyond that point. After data was written to the other end of the pipe, the pending read completed successfully. ThreadIsIoPending ================= The native behavior appears to be exposed through the ThreadIsIoPending thread information class. On native Windows: Ordinary event-backed overlapped named-pipe read while pending: NtQueryInformationThread(ThreadIsIoPending) status = STATUS_SUCCESS value = 1 length = sizeof(ULONG) After the operation is no longer pending: value = 0 An IOCP-backed operation that remains valid after the issuing thread terminates reports: value = 0 The IOCP operation also survives issuing-thread termination and later completes successfully. This matches the distinction Wine already makes when cancelling asynchronous operations during thread termination. I additionally tested the native ThreadIsIoPending query contract: * the output size must be exactly sizeof(ULONG); * invalid lengths return STATUS_INFO_LENGTH_MISMATCH; * a NULL output buffer with the correct size returns STATUS_ACCESS_VIOLATION; * THREAD_QUERY_INFORMATION access is required; * THREAD_QUERY_LIMITED_INFORMATION and SYNCHRONIZE alone are insufficient; * a valid handle to an already terminated thread succeeds and returns zero; * failed queries leave the output and ReturnLength untouched. Wine cause ========== Wine associates asynchronous operations with the issuing server thread. When a thread terminates, server/thread.c calls: cancel_terminating_thread_asyncs(thread) and pending async operations associated with that thread are normally cancelled. There is already an exception for operations associated with an I/O completion port and no event: if (async->completion && async->data.apc_context && !async->event) continue; Those operations survive thread termination, matching the native IOCP test above. The threadpool worker retirement code in dlls/ntdll/threadpool.c does not currently check whether terminating the worker would cancel pending I/O. Consequently an idle worker can be retired while an ordinary overlapped operation issued by one of its callbacks is still pending. Proposed fix ============ I have a two-patch series which: 1. Implements ThreadIsIoPending. The server uses the same predicate for both ThreadIsIoPending and cancel_terminating_thread_asyncs(), so the query reports exactly the asynchronous operations whose lifetime is tied to the queried thread. 2. Changes threadpool worker retirement so that, when a worker would otherwise retire, it queries ThreadIsIoPending and remains alive while such I/O is pending. Wine's existing 5000 ms worker timeout is left unchanged. Regression test =============== The patch series adds a deterministic regression test using a custom two-worker threadpool. The test: 1. creates two worker threads; 2. occupies one worker with a blocking callback; 3. has the second worker issue an overlapped named-pipe ReadFile(); 4. allows that callback to return while the read remains pending; 5. verifies that callback completion has occurred; 6. lowers the pool minimum and releases the blocking worker; 7. waits longer than Wine's worker-retirement timeout; 8. verifies that the worker owning the pending I/O is still alive; 9. writes to the other end of the pipe; 10. verifies that the read completes successfully. Without the worker-retirement fix, terminating the issuing worker cancels the read. With the fix, no-I/O workers retire normally while the I/O-owning worker is retained until the operation completes. Application impact ================== This was originally found with DxO PureRAW 4. PureRAW uses Qt named-pipe IPC. Under unmodified Wine the relevant connection is lost approximately five seconds after the callback that issued the pending read returns. A diagnostic Wine build with THREADPOOL_WORKER_TIMEOUT increased from 5000 to 60000 ms avoids the failure. The proposed patches do not change the timeout and do not contain any PureRAW-specific behavior; they implement the native threadpool/I/O semantics instead. Testing ======= Tested with: * native Windows 11 standalone conformance tests; * upstream Wine without the fix; * upstream Wine with the proposed patches; * DxO PureRAW 4; * dlls/kernel32/tests/thread.c; * dlls/ntdll/tests/threadpool.c. The existing ntdll/threadpool test suite has an unrelated pre-existing todo-success failure concerning timer merging. The proposed tests do not introduce an additional failure. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.