Currently, the signal queue is touched by the thread sig as well as
other threads that call sigaction_worker(). This potentially has
a possibility to destroy the signal queue chain. A possible worst
result may be a self-loop chain which causes infinite loop. With
this patch, lock()/unlock() are introduce to avoid such a situation.
Fixes: 474048c26edf ("* sigproc.cc (pending_signals::add): Just index directly into signal array rather than treating the array as a heap.")
Suggested-by: Corinna Vinschen <corinna@vinschen.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
The queue is cleaned up by removing the entries having si_signo == 0
while processing the queued signals, however, sigpacket::process() may
set si_signo in the queue to 0 of the entry already processed but not
succeed by calling sig_clear(). This patch ensures the sig_clear()
to remove the entry from the queue chain. For this purpose, the pointer
prev has been added to the sigpacket. This is to handle the following
case appropriately.
Consider the queued signal chain of:
A->B->C->D
without pointer prev. Assume that the pointer 'q' and 'qnext' point to
C, and process() is processing C. If B is cleared in process(), A->next
should be set to to C in sigpacket::clear().
Then, if process() for C succeeds, C should be removed from the queue,
so A->next should be set to D. However, we cannot do that because we do
not have the pointer to A in the while loop in wait_sig().
With the pointer prev, we can easily access A and C in sigpacket::clear()
as well as A and D in the while loop in wait_sig() using the pointer prev
and next without pursuing the chain.
Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html
Fixes: 9d2155089e87 ("(wait_sig): Define variable q to be the start of the signal queue. Just iterate through sigq queue, deleting processed or zeroed signals")
Reported-by: Christian Franke <Christian.Franke@t-online.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Previously, cygwin read pipe used non-blocking mode although non-
cygwin app uses blocking-mode by default. Despite this requirement,
if a cygwin app is executed from a non-cygwin app and the cygwin
app exits, read pipe remains on non-blocking mode because of the
commit fc691d0246b9. Due to this behaviour, the non-cygwin app
cannot read the pipe correctly after that. Similarly, if a non-
cygwin app is executed from a cygwin app and the non-cygwin app
exits, the read pipe mode remains on blocking mode although cygwin
read pipe should be non-blocking mode.
These bugs were provoked by pipe mode toggling between cygwin and
non-cygwin apps. To make management of pipe mode simpler, this
patch has re-designed the pipe implementation. In this new
implementation, both read and write pipe basically use only blocking
mode and the behaviour corresponding to the pipe mode is simulated
in raw_read() and raw_write(). Only when NtQueryInformationFile
(FilePipeLocalInformation) fails for some reasons, the raw_read()/
raw_write() cannot simulate non-blocking access. Therefore, the pipe
mode is temporarily changed to non-blocking mode.
Moreover, because the fact that NtSetInformationFile() in
set_pipe_non_blocking(true) fails with STATUS_PIPE_BUSY if the pipe
is not empty has been found, query handle is not necessary anymore.
This allows the implementation much simpler than before.
Addresses: https://github.com/git-for-windows/git/issues/5115
Fixes: fc691d0246b9 ("Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps.");
Reported-by: isaacag, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>, Ken Brown <kbrown@cornell.edu>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
