Skip to content

Commit

Permalink
Cygwin: pipe: Restore blocking mode of read pipe on close()/raw_read()
Browse files Browse the repository at this point in the history
If a cygwin app is executed from a non-cygwin app and the cygwin
app exits, the read pipe remains in the 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 remains in the blocking mode. With this
patch, the blocking mode of the read pipe is stored into a variable
was_blocking_read_pipe on set_pipe_non_blocking() when the cygwin
app starts and restored on close(). In addition, the pipe mode is
set to non-blocking mode in raw_read() if the mode is blocking
mode by referring the variable is_blocking_read_pipe as well.
is_blocking_read_pipe is a member of fhandler_pipe class and is set
by set_pipe_non_blocking(), so if other process sets the pipe mode
to blocking mode, the current process cannot know the pipe is
blocking mode. Therefore, is_blocking_read_pipe is also set on the
signal __SIGNONCYGCHLD, which is sent to the process group when
non-cygwin app is started.

Backported-from: c7fe29f5cb (Cygwin: pipe: Restore blocking mode of read pipe on close()/raw_read(), 2024-09-06)
Addresses: git-for-windows/git#5115
Fixes: fc691d0246b9 ("Cygwin: pipe: Make sure to set read pipe non-blocking for cygwin apps.");
Reported-by: isaacag, Johannes Schindelin <[email protected]>
Reviewed-by: Corinna Vinschen <[email protected]>, Ken Brown <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
  • Loading branch information
tyan0 authored and dscho committed Sep 25, 2024
1 parent 8854bb4 commit 9d0d6e1
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 8 deletions.
41 changes: 41 additions & 0 deletions winsup/cygwin/fhandler/pipe.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
NTSTATUS status;
IO_STATUS_BLOCK io;
FILE_PIPE_INFORMATION fpi;
bool was_blocking_read_pipe_new = was_blocking_read_pipe;

if (get_device () == FH_PIPER && nonblocking && !was_blocking_read_pipe)
{
status = NtQueryInformationFile (get_handle (), &io, &fpi, sizeof fpi,
FilePipeInformation);
if (NT_SUCCESS (status))
was_blocking_read_pipe_new =
(fpi.CompletionMode == FILE_PIPE_QUEUE_OPERATION);
}

fpi.ReadMode = FILE_PIPE_BYTE_STREAM_MODE;
fpi.CompletionMode = nonblocking ? FILE_PIPE_COMPLETE_OPERATION
Expand All @@ -62,6 +72,11 @@ fhandler_pipe::set_pipe_non_blocking (bool nonblocking)
FilePipeInformation);
if (!NT_SUCCESS (status))
debug_printf ("NtSetInformationFile(FilePipeInformation): %y", status);
else
{
was_blocking_read_pipe = was_blocking_read_pipe_new;
is_blocking_read_pipe = !nonblocking;
}
}

int
Expand Down Expand Up @@ -95,6 +110,8 @@ fhandler_pipe::init (HANDLE f, DWORD a, mode_t mode, int64_t uniq_id)
even with FILE_SYNCHRONOUS_IO_NONALERT. */
set_pipe_non_blocking (get_device () == FH_PIPER ?
true : is_nonblocking ());
was_blocking_read_pipe = false;

return 1;
}

Expand Down Expand Up @@ -289,6 +306,9 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
if (!len)
return;

if (is_blocking_read_pipe)
set_pipe_non_blocking (true);

DWORD timeout = is_nonblocking () ? 0 : INFINITE;
DWORD waitret = cygwait (read_mtx, timeout);
switch (waitret)
Expand Down Expand Up @@ -721,6 +741,8 @@ fhandler_pipe::close ()
CloseHandle (query_hdl);
if (query_hdl_close_req_evt)
CloseHandle (query_hdl_close_req_evt);
if (was_blocking_read_pipe)
set_pipe_non_blocking (false);
int ret = fhandler_base::close ();
ReleaseMutex (hdl_cnt_mtx);
CloseHandle (hdl_cnt_mtx);
Expand Down Expand Up @@ -1377,6 +1399,7 @@ fhandler_pipe::spawn_worker (int fileno_stdin, int fileno_stdout,
{
fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
pipe->set_pipe_non_blocking (false);
need_send_noncygchld_sig = true;
}

/* If multiple writers including non-cygwin app exist, the non-cygwin
Expand All @@ -1402,3 +1425,21 @@ fhandler_pipe::spawn_worker (int fileno_stdin, int fileno_stdout,
t->kill_pgrp (__SIGNONCYGCHLD);
}
}

void
fhandler_pipe::sigproc_worker (void)
{
cygheap_fdenum cfd (false);
while (cfd.next () >= 0)
if (cfd->get_dev () == FH_PIPEW)
{
fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
if (pipe->need_close_query_hdl ())
pipe->close_query_handle ();
}
else if (cfd->get_dev () == FH_PIPER)
{
fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
pipe->is_blocking_read_pipe = true;
}
}
3 changes: 3 additions & 0 deletions winsup/cygwin/local_includes/fhandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,8 @@ class fhandler_pipe: public fhandler_pipe_fifo
private:
HANDLE read_mtx;
pid_t popen_pid;
bool was_blocking_read_pipe;
bool is_blocking_read_pipe;
HANDLE query_hdl;
HANDLE hdl_cnt_mtx;
HANDLE query_hdl_proc;
Expand Down Expand Up @@ -1287,6 +1289,7 @@ class fhandler_pipe: public fhandler_pipe_fifo
}
static void spawn_worker (int fileno_stdin, int fileno_stdout,
int fileno_stderr);
static void sigproc_worker (void);
};

#define CYGWIN_FIFO_PIPE_NAME_LEN 47
Expand Down
9 changes: 1 addition & 8 deletions winsup/cygwin/sigproc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1475,14 +1475,7 @@ wait_sig (VOID *)
}
break;
case __SIGNONCYGCHLD:
cygheap_fdenum cfd (false);
while (cfd.next () >= 0)
if (cfd->get_dev () == FH_PIPEW)
{
fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
if (pipe->need_close_query_hdl ())
pipe->close_query_handle ();
}
fhandler_pipe::sigproc_worker ();
break;
}
if (clearwait && !have_execed)
Expand Down

0 comments on commit 9d0d6e1

Please sign in to comment.