[Bug 44816] New: Cygwin/ MSYS2 `script -e` exit status forwarding randomly returns zero for non zero child process

https://bugs.winehq.org/show_bug.cgi?id=44816

Bug ID: 44816
           Summary: Cygwin/MSYS2 `script -e` exit status forwarding
                    randomly returns zero for non zero child process
           Product: Wine
           Version: 3.4
          Hardware: x86-64
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ntdll
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Hello folks,

to track
https://github.com/wine-staging/wine-staging/tree/master/patches/ntdll-Deall...

Mentioned here:

https://wine-staging.com/news/2015-08-23-release-1.7.50.html

--- quote ---
Do not allow to deallocate thread stack for current thread (MSYS2, Wine Staging
Bug #241)
--- quote ---

After some archaeology I found the Cygwin mailing list discussion here:

https://cygwin.com/ml/cygwin/2015-09/msg00114.html

--- quote ---
> Ah, ok.  What OS does Wine emulate here?  Have a look at
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/cygth...
>
> The terminate_thread_frees_stack flag is set to false for XP/2003 and to
> true for any newer OS.  I guess this is a double-free because Wine's
> TerminateThread already freed the stack and Cygwin got the info it's
> supposedly running under XP/2003, so it tries to workaround the fact
> that TerminateThread on these systems didn't free the stack by themselves.

Wine emulates Windows XP here, I double checked Wine source code and I
can confirm Wine doesn't free the stack:
NtTerminateThread() -> abort_thread() -> terminate_thread()
https://github.com/wine-compholio/wine-patched/blob/8b3a785e97a7e28ff58731b5...

Cygwin reports wincap as wincap_xpsp2 which is also correct here.
> we need to know what address ret=61005767 is refering to.  addr2line would help.

As you point out, ret=61005767 is the call to VirtualFree() inside
cygthread::terminate_thread ()
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/cygth...

After combine the information we have so far, I think the problem is
clear now. It's not Cygwin's fault, it's Wine's fault. Wine's
WaitForSingleObject() needs to wait long enough after the
TerminateThread() call, so the target thread has enough time to finish
the system cleanup and return correct exit_code before Cygwin call
VirtualFree to free the stack. But in our current implementation
Wine's WaitForSingleObject returns too early, it shouldn't return
before the system thread cleanup done, as a result there is a race
here. We are thinking of a solution in Wine. Thanks again for the
great help!
--- quote ---

$ wine --version
wine-3.4

Regards