[PATCH] Cygwin: sigproc.cc: add comment
---
winsup/cygwin/sigproc.cc | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 2a9734f00..47352c213 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -44,7 +44,11 @@ char NO_COPY myself_nowait_dummy[1] = {'0'};
#define Static static NO_COPY
-/* All my children info. Avoid expensive constructor ops at DLL startup */
+/* All my children info. Avoid expensive constructor ops at DLL
+ startup.
+
+ This class can allocate memory. But there's no need to free it
+ because only one instance of the class is created per process. */
class child_procs {
#ifdef __i386__
static const int _NPROCS = 256;
--
2.28.0
Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
Hi Corinna, On Sat, 29 Aug 2020 04:25:54 +0900 Takashi Yano via Cygwin-patches wrote: > Hi Corinna, > > On Fri, 28 Aug 2020 15:45:03 +0200 > Corinna Vinschen wrote: > > Hi Takashi, > > > > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > > > Pseudo console generates escape sequences on execution of non-cygwin > > > apps. If the terminal does not support escape sequence, output will > > > be garbled. This patch prevents garbled output in dumb terminal by > > > disabling pseudo console. [...] > > > > Would you mind to encapsulate the TERM checks into a fhandler_pty_slave > > method so the TERM specific stuff is done in the fhandler code, not > > in spawn.cc? > > Thansk for the suggestion. I will submit v2 patch. What do you think of v3 patch attached? With this patch, terminal capability is checked by looking into terminfo database rather than just checking terminal name. This solution is more essential for the issue to be solved, I think. One downside of this solution, I noticed, is that tmux sets TERM to "screen", which does not have CSI6n, by default. As a result, pseudo console is disbled in tmux by default. Setting TERM, such as screen.xterm-256color, will solve the issue. -- Takashi Yano v3-0001-Cygwin-pty-Disable-pseudo-console-if-TERM-does-no.patch Description: Binary data
Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
On Sat, 29 Aug 2020 20:12:28 +0900 Takashi Yano via Cygwin-patches wrote: > Hi Corinna, > > On Sat, 29 Aug 2020 04:25:54 +0900 > Takashi Yano via Cygwin-patches wrote: > > Hi Corinna, > > > > On Fri, 28 Aug 2020 15:45:03 +0200 > > Corinna Vinschen wrote: > > > Hi Takashi, > > > > > > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > > > > Pseudo console generates escape sequences on execution of non-cygwin > > > > apps. If the terminal does not support escape sequence, output will > > > > be garbled. This patch prevents garbled output in dumb terminal by > > > > disabling pseudo console. > [...] > > > > > > Would you mind to encapsulate the TERM checks into a fhandler_pty_slave > > > method so the TERM specific stuff is done in the fhandler code, not > > > in spawn.cc? > > > > Thansk for the suggestion. I will submit v2 patch. > > What do you think of v3 patch attached? With this patch, > terminal capability is checked by looking into terminfo > database rather than just checking terminal name. This > solution is more essential for the issue to be solved, > I think. > > One downside of this solution, I noticed, is that tmux > sets TERM to "screen", which does not have CSI6n, by > default. As a result, pseudo console is disbled in tmux > by default. Setting TERM, such as screen.xterm-256color, > will solve the issue. Attached is the v4 patch. Small bug was fixed. -- Takashi Yano v4-0001-Cygwin-pty-Disable-pseudo-console-if-TERM-does-no.patch Description: Binary data
[PATCH] Cygwin: Remove waitloop argument from try_to_debug()
Currently, when using CYGWIN='error_start=dumper', the core dump written
in response to an exception is non-deterministic, as the faulting
process isn't stopped while the dumper is started (it even seems
possible in theory that the faulting process could have exited before
the dumper process attaches).
Remove the waitloop argument, only used in this case, so the faulting
process busy-waits until the dump starts.
Code archeology to determine why the code is this way didn't really turn
up any answers, but this seems a low-risk change, as this only changes
the behaviour when:
- a debugger isn't already attached
- an error_start is specified in CYGWIN env var
- an exception has occured which will be translated to a signal
Future work: This probably can be further simplified to make it
completely synchronous by waiting for the dumper process to exit. This
would avoid the race condition of the dumper attaching and detaching
before we get around to checking for that (which we try to work around
by juggling thread priorities), and the failure state where the dumper
doesn't attach and we spin indefinitely.
---
winsup/cygwin/exceptions.cc | 8 ++--
winsup/cygwin/winsup.h | 2 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index bb7704f94..3ed2db1eb 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -461,10 +461,8 @@ cygwin_stackdump ()
exc.dumpstack ();
}
-#define TIME_TO_WAIT_FOR_DEBUGGER 1
-
extern "C" int
-try_to_debug (bool waitloop)
+try_to_debug ()
{
if (!debugger_command)
return 0;
@@ -537,8 +535,6 @@ try_to_debug (bool waitloop)
system_printf ("Failed to start debugger, %E");
else
{
- if (!waitloop)
- return dbg;
SetThreadPriority (GetCurrentThread (), THREAD_PRIORITY_IDLE);
while (!being_debugged ())
Sleep (1);
@@ -812,7 +808,7 @@ exception::handle (EXCEPTION_RECORD *e, exception_list
*frame, CONTEXT *in,
if (exit_state >= ES_SIGNAL_EXIT
&& (NTSTATUS) e->ExceptionCode != STATUS_CONTROL_C_EXIT)
api_fatal ("Exception during process exit");
- else if (!try_to_debug (0))
+ else if (!try_to_debug ())
rtl_unwind (frame, e);
else
{
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 79844cb87..0ffd8c5af 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -190,7 +190,7 @@ void close_all_files (bool = false);
/* debug_on_trap support. see exceptions.cc:try_to_debug() */
extern "C" void error_start_init (const char*);
-extern "C" int try_to_debug (bool waitloop = 1);
+extern "C" int try_to_debug ();
void ld_preload ();
void fixup_hooks_after_fork ();
--
2.28.0
Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
On Sat, 29 Aug 2020 22:14:20 +0900 Takashi Yano via Cygwin-patches wrote: > On Sat, 29 Aug 2020 20:12:28 +0900 > Takashi Yano via Cygwin-patches wrote: > > Hi Corinna, > > > > On Sat, 29 Aug 2020 04:25:54 +0900 > > Takashi Yano via Cygwin-patches wrote: > > > Hi Corinna, > > > > > > On Fri, 28 Aug 2020 15:45:03 +0200 > > > Corinna Vinschen wrote: > > > > Hi Takashi, > > > > > > > > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > > > > > Pseudo console generates escape sequences on execution of non-cygwin > > > > > apps. If the terminal does not support escape sequence, output will > > > > > be garbled. This patch prevents garbled output in dumb terminal by > > > > > disabling pseudo console. > > [...] > > > > > > > > Would you mind to encapsulate the TERM checks into a fhandler_pty_slave > > > > method so the TERM specific stuff is done in the fhandler code, not > > > > in spawn.cc? > > > > > > Thansk for the suggestion. I will submit v2 patch. > > > > What do you think of v3 patch attached? With this patch, > > terminal capability is checked by looking into terminfo > > database rather than just checking terminal name. This > > solution is more essential for the issue to be solved, > > I think. > > > > One downside of this solution, I noticed, is that tmux > > sets TERM to "screen", which does not have CSI6n, by > > default. As a result, pseudo console is disbled in tmux > > by default. Setting TERM, such as screen.xterm-256color, > > will solve the issue. > > Attached is the v4 patch. Small bug was fixed. Bug fixed again. v5 patch attached. -- Takashi Yano v5-0001-Cygwin-pty-Disable-pseudo-console-if-TERM-does-no.patch Description: Binary data
Re: [PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.
On Sun, 30 Aug 2020 05:25:06 +0900 Takashi Yano via Cygwin-patches wrote: > On Sat, 29 Aug 2020 22:14:20 +0900 > Takashi Yano via Cygwin-patches wrote: > > On Sat, 29 Aug 2020 20:12:28 +0900 > > Takashi Yano via Cygwin-patches wrote: > > > Hi Corinna, > > > > > > On Sat, 29 Aug 2020 04:25:54 +0900 > > > Takashi Yano via Cygwin-patches wrote: > > > > Hi Corinna, > > > > > > > > On Fri, 28 Aug 2020 15:45:03 +0200 > > > > Corinna Vinschen wrote: > > > > > Hi Takashi, > > > > > > > > > > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote: > > > > > > Pseudo console generates escape sequences on execution of non-cygwin > > > > > > apps. If the terminal does not support escape sequence, output will > > > > > > be garbled. This patch prevents garbled output in dumb terminal by > > > > > > disabling pseudo console. > > > [...] > > > > > > > > > > Would you mind to encapsulate the TERM checks into a > > > > > fhandler_pty_slave > > > > > method so the TERM specific stuff is done in the fhandler code, not > > > > > in spawn.cc? > > > > > > > > Thansk for the suggestion. I will submit v2 patch. > > > > > > What do you think of v3 patch attached? With this patch, > > > terminal capability is checked by looking into terminfo > > > database rather than just checking terminal name. This > > > solution is more essential for the issue to be solved, > > > I think. > > > > > > One downside of this solution, I noticed, is that tmux > > > sets TERM to "screen", which does not have CSI6n, by > > > default. As a result, pseudo console is disbled in tmux > > > by default. Setting TERM, such as screen.xterm-256color, > > > will solve the issue. > > > > Attached is the v4 patch. Small bug was fixed. > > Bug fixed again. v5 patch attached. v6: Refactor the code a little. -- Takashi Yano v6-0001-Cygwin-pty-Disable-pseudo-console-if-TERM-does-no.patch Description: Binary data
