Hi Takashi, On Thu, 3 Apr 2025, Takashi Yano wrote:
> Previously, the callback registered by pthread_atfork() is called > before _my_tls.fixup_after_fork(). This causes misbehaviour if the > callback uses tls related functions. Due to this problem, subprocess > of cmake (> 3.29.x) sometimes fails after the commit 7ed9adb356df. > The commit 7ed9adb356df triggers this issue, but it is not the issue > of that patch. This patch moves the pthread::atforkchild() at the > end of the fork::child(). That's a good initial draft. Let's expand this a bit with the information you provided in https://github.com/msys2/msys2-runtime/issues/272#issuecomment-2775398477: The event handle signal_arrived is initialized in _cygtls::fixup_after_fork() but pthread::atforkchild() calls user callbacks before _cygtls::fixup_after_fork(). This was the cause of invalid handle. cmake uses thread_atfork() and print something in the user callback. fhandler_fifo_pipe::raw_write() calls cygwait() which uses _cygtls::signal_arrived so pthread::atforkchild() should not be called before _cygtls::fixup_after_fork(). I still have a question that I would like to be answered in the commit message, too: If `signal_arrived` is only initialized in `fixup_after_fork()` but user callbacks that use this are called by `atforkchild()`, why did this not trigger _all the time_ before your reordering of the calls? Ciao, Johannes > > Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257800.html > Addresses: https://github.com/msys2/msys2-runtime/issues/272 > Fixes: f02b22dcee17 ("* fork.cc (frok::child): Change order of cleanup prior > to return.") > Reported-by: Christoph Reiter <reiter.christ...@gmail.com> > Reviewed-by: Jeremy Drake <cyg...@jdrake.com> > Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> > --- > winsup/cygwin/fork.cc | 2 +- > winsup/cygwin/release/3.6.1 | 4 ++++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc > index 783971b76..f88acdbbf 100644 > --- a/winsup/cygwin/fork.cc > +++ b/winsup/cygwin/fork.cc > @@ -187,7 +187,6 @@ frok::child (volatile char * volatile here) > > ForceCloseHandle1 (fork_info->forker_finished, forker_finished); > > - pthread::atforkchild (); > cygbench ("fork-child"); > ld_preload (); > fixup_hooks_after_fork (); > @@ -199,6 +198,7 @@ frok::child (volatile char * volatile here) > CloseHandle (hParent); > hParent = NULL; > cygwin_finished_initializing = true; > + pthread::atforkchild (); > return 0; > } > > diff --git a/winsup/cygwin/release/3.6.1 b/winsup/cygwin/release/3.6.1 > index 07a29ecce..5a15642b8 100644 > --- a/winsup/cygwin/release/3.6.1 > +++ b/winsup/cygwin/release/3.6.1 > @@ -31,3 +31,7 @@ Fixes: > > - Return EMFILE when opening /dev/ptmx too many times. > Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257786.html > + > +- Move pthread::atforkchild() at the end of fork::child(). > + Addresses: https://cygwin.com/pipermail/cygwin/2025-March/257800.html > + Addresses: https://github.com/msys2/msys2-runtime/issues/272 > -- > 2.45.1 > >
