Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: gdb-patches@sourceware.org
Subject: Re: [RFC] control-c handling on Windows...
Date: Sat, 10 May 2008 13:10:00 -0000	[thread overview]
Message-ID: <20080510013336.GC28890@adacore.com> (raw)
In-Reply-To: <20080509202942.GA4386@ednor.casa.cgf.cx>

[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]

Hi Chris,

> Thanks for the analysis.  I've talked to Corinna about this since she
> was responsible for the original patch in question.  I think her patch
> still makes sense so I guess your change is required although I can't
> shake the feeling that there is some CreateProcess setting that we're
> missing to deal with this behavior.

I had a second look at the issue that Corinna was facing. I am not
entirely sure that I'm seeing the exact same problem, but the symptoms
are the same (gdbtui's process goes into background).

As far as I can tell, here is the sequence of events that lead to
the problem:

  1. win32_create_inferior creates the process, and then calls
     do_initial_win32_stuff.
  2. do_initial_win32_stuff then gives the terminal to the inferior,
     and then does a wait_for_inferior () until we get the expected
     SIGTRAP.
  3. wait_for_inferior leads us to calling win32_wait which calls
     which in turn calls get_win32_debug_event.
  4. As expected, we get our CREATE_PROCESS_DEBUG_EVENT. As a result,
     we add this new pid to the list of threads calling
     win32_add_thread.
  5. win32_add_thread calls add_thread as well, which calls
     add_thread_with_info. This is where things get interesting:

          if (print_thread_events)
            printf_unfiltered (_("[New %s]\n"), target_pid_to_str (ptid));

When I remove this printf_unfiltered, the startup phase completes fine.
I was going to propose that we therefore remove the target_terminal_ours
out of win32_wait and bracket the call to add_thread with
target_terminal_ours/target_terminal_inferior. Things seemed to work
relatively fine, except that I occasionally got the wrong behavior
when pressing C-c. Also, the new suggestion would not take care of
the multiple DEBUG traces that we can get GDB to print.

As far as I can tell, it almost looks like the SIGINT is buffered
until we switch the terminal to the debugger.  Using printfs,
the sequence I see is: we see the new thread, we switch the terminal
to ours to print the new thread event, we receive the SIGINT...
I just don't understand.

> But, if your patch fixes the problem I'm ok with checking it in.

I think that this is probably the easiest to solve all problems.
Here is the same patch, but with a comment explaining why we temporarily
ignore SIGINT...

2008-05-10  Joel Brobecker  <brobecker@adacore.com>

        * win32-nat.c (win32_wait): Ignore SIGINT while waiting for
        a debug event.

I thought you might have some comments/suggestions on the comment,
so I'll wait for your OK before checking it in.

Thanks,
-- 
Joel

[-- Attachment #2: win32-nat.c.diff --]
[-- Type: text/plain, Size: 1046 bytes --]

Index: win32-nat.c
===================================================================
--- win32-nat.c	(revision 130731)
+++ win32-nat.c	(working copy)
@@ -1467,7 +1467,20 @@ win32_wait (ptid_t ptid, struct target_w
 
   while (1)
     {
-      int retval = get_win32_debug_event (pid, ourstatus);
+      int retval;
+      void (*ofunc) (int);
+
+      /* For some reason, even when the terminal is owned by the inferior,
+         pressing control-c in the debugger window sometimes leads to
+         the debugger getting the associated SIGINT, which is unexpected.
+         In fact, both the inferior and the debugger get this signal.
+         To avoid getting this signal in the debugger, we temporarily
+         ignore SIGINT while waiting for debug events.  However, this
+         might be a symptom of a problem in our terminal settings.  */
+      ofunc = signal (SIGINT, SIG_IGN);
+      retval = get_win32_debug_event (pid, ourstatus);
+      signal (SIGINT, ofunc);
+
       if (retval)
 	return pid_to_ptid (retval);
       else

  reply	other threads:[~2008-05-10  1:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-08  9:46 Joel Brobecker
2008-05-10  4:34 ` Christopher Faylor
2008-05-10 13:10   ` Joel Brobecker [this message]
2008-05-10 15:31     ` Christopher Faylor
2008-05-10 22:28       ` Joel Brobecker
2008-05-11  5:30         ` Christopher Faylor
2008-05-11 13:31         ` Joel Brobecker
2008-05-11 13:46           ` Eli Zaretskii
2008-05-11 13:58           ` Christopher Faylor
2008-05-11 21:26             ` Joel Brobecker
2008-05-17 11:38               ` Christopher Faylor
2008-05-21  0:34                 ` Joel Brobecker
2008-05-21  3:19                   ` Christopher Faylor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080510013336.GC28890@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox