Hi Takashi, the commit message you proposed at <https://github.com/msys2/msys2-runtime/issues/272#issuecomment-2777433637> looks better.
Is Jeremy's guess "if raw_write doesn't need to wait (ie, there's room in the pipe for the write) it doesn't hit the signal stuff" correct? If so, it would be good to add that part to the commit message because the commit would otherwise still be incomplete. Also: referring to 7ed9adb356df may be technically correct, but human readers have a much easier time when that shortened commit OID is accompanied by some human-readable information, such as the string obtained via `git show --format=reference` (see <https://git-scm.com/docs/git-show#_pretty_formats>). In general, I would like to encourage you to err on the verbose side in commit messages: whenever something seems so obvious to you that you do not want to bother with writing it down, do first ask yourself if this would have been obvious to you even if you had _not_ stared at the code for days and even if you had not authored the commits you referenced. Or if you would remember those details in six months if you were having the most delightful no-internet vacation of your life during those six months. If there is _any_ doubt, then write it down. As the saying goes: If you don't write it down, it never happened. (And that goes for analyzing bugs, and figuring out code flows, too.) Ciao, Johannes P.S.: Sorry for top-posting, I'm sending this from a phone. -------- Original Message -------- From: Takashi Yano <takashi.y...@nifty.ne.jp> Sent: April 4, 2025 3:58:39 AM GMT+02:00 To: cygwin-patches@cygwin.com Subject: Re: [PATCH] Cygwin: fork: Call pthread::atforkchild () after other initializations On Thu, 3 Apr 2025 10:01:07 -0700 (PDT) Jeremy Drake wrote: > On Thu, 3 Apr 2025, Johannes Schindelin wrote: > > > 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? > > Based on my recollections, Takashi probably knows better > > 1) there has to be a pthread_atfork child callback registered > 2) this callback has to call raw_write > 3) raw_write now calls cygwait (which is now reenterancy-safe due to other > fallout from this) > 4) cygwait allows signals to be processed, so needs the signal-handling > stuff to be properly initialized. > > I'm guessing, if raw_write doesn't need to wait (ie, there's room in the > pipe for the write) it doesn't hit the signal stuff. Thanks for the explanation. Actually, cygwait() waits for a mutex at the beginning of raw_write(). This is introduced by the commit 7ed9adb356df, so the bug does not affect before that commit. > But I get your request for explaining the scenario in the commit message. I'll add the descriptions requested by Johannes before push. Thanks!
