[PATCH] Cygwin: pty: Add missing guard for close_pseudoconsole().
- This patch adds a missing mutex guard for close_pseudoconsole() call when GDB exits. --- winsup/cygwin/fhandler_tty.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index 530321513..9c03e09a7 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -177,7 +177,9 @@ atexit_func (void) input_available_event); ReleaseMutex (ptys->input_mutex); } + WaitForSingleObject (ptys->pcon_mutex, INFINITE); ptys->close_pseudoconsole (ttyp, force_switch_to); + ReleaseMutex (ptys->pcon_mutex); break; } CloseHandle (h_gdb_process); -- 2.31.1
[PATCH] Cygwin: pty: Fix fallback processing in setup_pseudoconsole().
- Currently, the fallback processing in setup_pseudoconsole() when helper process error occurs does not work properly. This patch fixes the issue. --- winsup/cygwin/fhandler_tty.cc | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index e4480771b..530321513 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -3226,15 +3226,15 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon) if (wait_result == WAIT_OBJECT_0) break; if (wait_result != WAIT_TIMEOUT) - goto cleanup_helper_process; + goto cleanup_helper_with_hello; DWORD exit_code; if (!GetExitCodeProcess (pi.hProcess, &exit_code)) - goto cleanup_helper_process; + goto cleanup_helper_with_hello; if (exit_code == STILL_ACTIVE) continue; if (exit_code != 0 || WaitForSingleObject (hello, 500) != WAIT_OBJECT_0) - goto cleanup_helper_process; + goto cleanup_helper_with_hello; break; } CloseHandle (hello); @@ -3349,6 +3349,10 @@ skip_create: return true; +cleanup_helper_with_hello: + CloseHandle (hello); + CloseHandle (pi.hThread); + goto cleanup_helper_process; cleanup_pcon_in: CloseHandle (hpConIn); cleanup_helper_process: @@ -3358,10 +3362,10 @@ cleanup_helper_process: goto skip_close_hello; cleanup_event_and_pipes: CloseHandle (hello); +skip_close_hello: get_ttyp ()->pcon_start = false; get_ttyp ()->pcon_start_pid = 0; get_ttyp ()->pcon_activated = false; -skip_close_hello: CloseHandle (goodbye); CloseHandle (hr); CloseHandle (hw); -- 2.31.1
[PATCH] Cygwin: pty: Additional race issue fix regarding pseudo console.
- In commit bb93c6d7, the race issue was not completely fixed. In the pseudo console inheritance, if the destination process to which the ownership of pseudo console switches, is found but exits before switching, the inheritance fails. Currently, this extremely rarely happens. This patch fixes the issue. --- winsup/cygwin/fhandler_tty.cc | 47 +++ 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index d44728795..e4480771b 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -64,6 +64,8 @@ struct pipe_reply { extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */ +inline static bool pcon_pid_alive (DWORD pid); + DWORD fhandler_pty_common::get_console_process_id (DWORD pid, bool match, bool cygwin, bool stub_only) @@ -90,20 +92,18 @@ fhandler_pty_common::get_console_process_id (DWORD pid, bool match, else { pinfo p (cygwin_pid (list[i])); - if (!!p && p->dwProcessId && p->exec_dwProcessId - && p->dwProcessId != p->exec_dwProcessId) + if (!!p && p->exec_dwProcessId) { - res_pri = list[i]; + res_pri = stub_only ? p->exec_dwProcessId : list[i]; break; } - if (!!p && !res) + if (!p && !res && pcon_pid_alive (list[i]) && stub_only) + res = list[i]; + if (!!p && !res && !stub_only) res = list[i]; } } - if (stub_only) -return res_pri; - else -return res_pri ?: res; + return res_pri ?: res; } static bool isHybrid; @@ -3384,35 +3384,17 @@ fallback: void fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to) { - DWORD switch_to_stub = 0, switch_to = 0; - DWORD new_pcon_pid = 0; + DWORD switch_to = 0; if (force_switch_to) { - switch_to_stub = force_switch_to; - new_pcon_pid = force_switch_to; + switch_to = force_switch_to; ttyp->setpgid (force_switch_to + MAX_PID); } else if (pcon_pid_self (ttyp->pcon_pid)) { /* Search another process which attaches to the pseudo console */ DWORD current_pid = myself->exec_dwProcessId ?: myself->dwProcessId; - switch_to = get_console_process_id (current_pid, false, true); - if (switch_to) - { - pinfo p (cygwin_pid (switch_to)); - if (p) - { - if (p->exec_dwProcessId) - switch_to_stub = p->exec_dwProcessId; - new_pcon_pid = p->exec_dwProcessId; - } - } - else - { - switch_to = get_console_process_id (current_pid, false, false); - if (switch_to) - new_pcon_pid = switch_to; - } + switch_to = get_console_process_id (current_pid, false, true, true); } if (ttyp->pcon_activated) { @@ -3420,7 +3402,6 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to) ttyp->previous_output_code_page = GetConsoleOutputCP (); if (pcon_pid_self (ttyp->pcon_pid)) { - switch_to = switch_to_stub ?: switch_to; if (switch_to) { /* Change pseudo console owner to another process */ @@ -3454,7 +3435,7 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to) CloseHandle (ttyp->h_pcon_conhost_process); CloseHandle (ttyp->h_pcon_in); CloseHandle (ttyp->h_pcon_out); - ttyp->pcon_pid = new_pcon_pid; + ttyp->pcon_pid = switch_to; ttyp->h_pcon_write_pipe = new_write_pipe; ttyp->h_pcon_condrv_reference = new_condrv_reference; ttyp->h_pcon_conhost_process = new_conhost_process; @@ -3513,8 +3494,8 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to) } else if (pcon_pid_self (ttyp->pcon_pid)) { - if (switch_to_stub) - ttyp->pcon_pid = new_pcon_pid; + if (switch_to) + ttyp->pcon_pid = switch_to; else { ttyp->pcon_pid = 0; -- 2.31.1
Re: [PATCH] Use automake (v5)
On 20/04/2021 21:13, Jon Turney wrote: For ease of reviewing, this patch doesn't contain changes to generated files which would be made by running ./autogen.sh. Sorry about getting distracted from this. To summarize what I believe were the outstanding issues with v3 [1]: [1] https://cygwin.com/pipermail/cygwin-patches/2020q4/010827.html * 'INCLUDES' is the old name for 'AM_CPPFLAGS' warning from autogen.sh I plan to clean this up in a future patch * 'ps$(EXEEXT)' previously defined' warning from autogen.sh It seems to be a shortcoming of automake that there's no way to suppress just that warning. One possible solution is build ps.exe with a different name and rename it while installing, but I think that is counter-productive (in the sense that it trades this warning for making the build more complex to understand) * some object files are in a unexpected places in the build file hierarchy (compared to naive expectations and/or the non-automake build) I'm not sure if this is merely an aesthetic issue, or if there are problems this causes.
[PATCH] Use automake (v5)
For ease of reviewing, this patch doesn't contain changes to generated files which would be made by running ./autogen.sh. v2: * Include tzmap.h in BUILT_SOURCES * Make per-file flags appear after user-supplied CXXFLAGS, so they can override optimization level. * Correct .o files used to define symbols exported by libm.a * Drop gcrt0.o mistakenly included in libgmon.a * Add missing line continuations in GMON_FILES value v3: * use per-file flags for .c compilation * override C{XX,}FLAGS, as they are set on the command line by top-level make v4: * Drop -Wno-error=write-strings from path_testsuite CXXFLAGS v5: * Update for changes in master - Add -fno-threadsafe-statics to CXX flags - Add hypotl.cc - Remove fenv.cc (in favour of newlib), add fenv.c stub - Add proc.5 manpage rules --- winsup/Makefile.am | 19 + winsup/Makefile.am.common | 15 + winsup/Makefile.common | 51 -- winsup/autogen.sh | 1 + winsup/configure.ac| 21 +- winsup/cygserver/Makefile.am | 58 ++ winsup/cygwin/Makefile.am | 770 + winsup/cygwin/config.h.in | 2 +- winsup/doc/Makefile.am | 162 ++ winsup/testsuite/Makefile.am | 64 ++ winsup/testsuite/config/default.exp| 4 +- winsup/testsuite/cygrun/Makefile.am| 21 + winsup/testsuite/winsup.api/winsup.exp | 6 +- winsup/utils/Makefile.am | 81 +++ winsup/utils/mingw/Makefile.am | 50 ++ 15 files changed, 1266 insertions(+), 59 deletions(-) create mode 100644 winsup/Makefile.am create mode 100644 winsup/Makefile.am.common delete mode 100644 winsup/Makefile.common create mode 100644 winsup/cygserver/Makefile.am create mode 100644 winsup/cygwin/Makefile.am create mode 100644 winsup/doc/Makefile.am create mode 100644 winsup/testsuite/Makefile.am create mode 100644 winsup/testsuite/cygrun/Makefile.am create mode 100644 winsup/utils/Makefile.am create mode 100644 winsup/utils/mingw/Makefile.am diff --git a/winsup/Makefile.am b/winsup/Makefile.am new file mode 100644 index 0..067f74688 --- /dev/null +++ b/winsup/Makefile.am @@ -0,0 +1,19 @@ +# Makefile.am for winsup stuff +# +# This file is part of Cygwin. +# +# This software is a copyrighted work licensed under the terms of the +# Cygwin license. Please consult the file "CYGWIN_LICENSE" for +# details. + +# This makefile requires GNU make. + +cygdocdir = $(datarootdir)/doc/Cygwin + +cygdoc_DATA = \ + CYGWIN_LICENSE \ + COPYING + +SUBDIRS = cygwin cygserver doc utils testsuite + +cygserver utils testsuite: cygwin diff --git a/winsup/Makefile.am.common b/winsup/Makefile.am.common new file mode 100644 index 0..884194df2 --- /dev/null +++ b/winsup/Makefile.am.common @@ -0,0 +1,15 @@ +# Makefile.am.common - common definitions for the winsup directory +# +# This file is part of Cygwin. +# +# This software is a copyrighted work licensed under the terms of the +# Cygwin license. Please consult the file "CYGWIN_LICENSE" for +# details. + +flags_common=-Wall -Wstrict-aliasing -Wwrite-strings -fno-common -pipe -fbuiltin -fmessage-length=0 + +# compiler flags commonly used (but not for MinGW compilation, because they +# include the Cygwin header paths via @INCLUDES@) + +cxxflags_common=$(INCLUDES) -fno-rtti -fno-exceptions -fno-use-cxa-atexit $(flags_common) +cflags_common=$(INCLUDES) $(flags_common) diff --git a/winsup/Makefile.common b/winsup/Makefile.common deleted file mode 100644 index 3141bd111..0 --- a/winsup/Makefile.common +++ /dev/null @@ -1,51 +0,0 @@ -# Makefile.common - common definitions for the winsup directory -# -# This file is part of Cygwin. -# -# This software is a copyrighted work licensed under the terms of the -# Cygwin license. Please consult the file "CYGWIN_LICENSE" for -# details. - -define justdir -$(patsubst %/,%,$(dir $1)) -endef - -define libname -$(realpath $(shell ${CC} --print-file-name=$1 $2)) -endef - -export PATH:=${winsup_srcdir}:${PATH} - -# Allow CFLAGS=-O,-g to control CXXFLAGS too -opt=$(filter -O%,${CFLAGS}) $(filter -g%,${CFLAGS}) -override CXXFLAGS:=${filter-out -g%,$(filter-out -O%,${CXXFLAGS})} ${opt} - -cflags_common:=-Wall -Wstrict-aliasing -Wwrite-strings -fno-common -pipe -fbuiltin -fmessage-length=0 -COMPILE.cc=${CXX} ${INCLUDES} ${CXXFLAGS} -fno-rtti -fno-exceptions -fno-use-cxa-atexit ${cflags_common} -COMPILE.c=${CC} ${INCLUDES} ${CFLAGS} ${cflags_common} - -top_srcdir:=$(call justdir,${winsup_srcdir}) -top_builddir:=$(call justdir,${target_builddir}) - -cygwin_build:=${target_builddir}/winsup/cygwin -newlib_build:=${target_builddir}/newlib - -VPATH:=${srcdir} - -.SUFFIXES: -.SUFFIXES: .c .cc .def .S .a .o .d .s .E - -%.o: %.cc - $(strip ${COMPILE.cc} -c -o $@ $<) - -%.o: %.c - $(strip ${COMPILE.c} -c -o $@ $<) - -%.E: %.cc - $(strip ${COMPILE.cc} -E -dD -o $@ $<) - -%.E: %.c - $(strip ${COMPILE.c} -E
RE: [PATCH 0/2] Fix race issues.
Corinna Vinschen wrote: > On Apr 19 19:30, Takashi Yano wrote: > > Takashi Yano (2): > > Cygwin: console: Fix race issue regarding cons_master_thread(). > > Cygwin: pty: Fix race issue in inheritance of pseudo console. > > > > winsup/cygwin/fhandler_console.cc | 10 ++- > > winsup/cygwin/fhandler_tty.cc | 108 ++ > > winsup/cygwin/tty.cc | 15 ++--- > > winsup/cygwin/tty.h | 2 +- > > 4 files changed, 77 insertions(+), 58 deletions(-) > > > > -- > > 2.31.1 > > Pushed. Armed with this morning's Cygwin snapshot, OCaml builds again as well. Many thanks! David
Re: [PATCH] Cygwin: pty: Make rlwrap work with cmd.exe.
On Apr 20 05:51, Takashi Yano wrote: > On Mon, 19 Apr 2021 16:19:09 +0200 > Corinna Vinschen wrote: > > Hi Takashi, > > > > On Apr 19 20:51, Takashi Yano wrote: > > > - After the commit 919dea66, "rlwrap cmd" fails to start pseudo > > > console. This patch fixes the issue. > > > --- > > > winsup/cygwin/fhandler_tty.cc | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc > > > index ba9f4117f..d44728795 100644 > > > --- a/winsup/cygwin/fhandler_tty.cc > > > +++ b/winsup/cygwin/fhandler_tty.cc > > > @@ -1170,6 +1170,8 @@ fhandler_pty_slave::reset_switch_to_pcon (void) > > > } > > >if (isHybrid) > > > return; > > > + if (get_ttyp ()->pcon_start) > > > +return; > > >WaitForSingleObject (pcon_mutex, INFINITE); > > >if (!pcon_pid_self (get_ttyp ()->pcon_pid) > > >&& pcon_pid_alive (get_ttyp ()->pcon_pid)) > > > -- > > > 2.31.1 > > > > this patch doesn't apply. Does it depend on the race issue fixes? > > Yes. This depends on the race issue patches. > Please apply this patch after the race issue patches. > > Thanks. Pushed. Thanks, Corinna
Re: [PATCH 0/2] Fix race issues.
On Apr 19 19:30, Takashi Yano wrote: > Takashi Yano (2): > Cygwin: console: Fix race issue regarding cons_master_thread(). > Cygwin: pty: Fix race issue in inheritance of pseudo console. > > winsup/cygwin/fhandler_console.cc | 10 ++- > winsup/cygwin/fhandler_tty.cc | 108 ++ > winsup/cygwin/tty.cc | 15 ++--- > winsup/cygwin/tty.h | 2 +- > 4 files changed, 77 insertions(+), 58 deletions(-) > > -- > 2.31.1 Pushed. Thanks, Corinna