Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA 2/2][master only] gdb/windows-nat.c: Get rid of main_thread_id global
Date: Wed, 17 Apr 2019 22:17:00 -0000	[thread overview]
Message-ID: <20190417221729.GA5839@adacore.com> (raw)
In-Reply-To: <83wojseb9c.fsf@gnu.org>

> > If I understand you correctly, you want to preserve the global
> > so we can avoid those notifications. In my opinion, doing that
> > just for the purpose of filtering one notification is simply
> > not worth the trouble of keeping a global around.  Generally speaking,
> > we tend to try to avoid globals as much as we can, as they are hard to
> > track, and makes the code harder to understand and maintain.
> > Modifying the logic to use the main_thread_id global instead of
> > the kind of event is a step backwards, in my opinion.
> 
> If the only problem is having the global variable, we could mark the
> main thread in some other way, that won't need the global.
> 
> But yes, I think not announcing the main thread is a nice feature, and
> losing it due to the mess that happens at program exit (something to
> which most users won't pay attention anyway) would be a pity.  I'd
> even agree to announcing the exit of the main thread, if we cannot
> come up with a better solution, if that could allow us avoid its
> announcement at program startup.
> 
> The use case I have in mind is debugging a single-threaded program.
> It is jarring enough, when debugging such a program, to see GDB
> announcing threads started by the OS.  It would be nice if we could
> avoid announcing the main thread.

Personally, I feel the other way, especially since we always get
all sorts of random temporary thread notifications polutting the
output, that I don't mind the extra notification. And that way,
I know when each thread is actually terminated, and with what
error code.

But if you and others feel differently, I can switch my approach:

  - patch #1 (master+8.3): Just fix the crash, and nothing more.

        This means that, on 8.3, thread creation and exit notifications
        can be slightly off; this is considered an acceptable known
        problem, and is not a regression.

        This is actually what we've put in AdaCore's GDB as a first
        step to see we had any issue after that, and we did not.

  - patch #2 (master only): Remove uses of the global

        But we keep the filtering of the "main" thread. We still
        the known problem that the thread-exit notifications might
        be silenced for the wrong thread. Not a regression.

  - patch #3 (master only): Tag the main thread, and silence
    the thread-exit notification of the thread that got tagged.
    Depending on the implementation, this patch would potentially
    be acceptable for 8.3, but might need extra work to apply.
    I think we've had enough problems with 8.3 as it is, I don't
    want to add more things to do, so I vote OUT for 8.3.

I don't think I have time for that before the weekend or next week,
though.

-- 
Joel


  reply	other threads:[~2019-04-17 22:17 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-16 22:33 Windows native GDB event handling enhancement Joel Brobecker
2019-04-16 22:33 ` [RFA 1/2][master+8.3] (Windows) fix thr != nullptr assert failure in delete_thread_1 Joel Brobecker
2019-04-18 14:52   ` Pedro Alves
2019-04-18 15:04     ` Pedro Alves
2019-04-16 22:33 ` [RFA 2/2][master only] gdb/windows-nat.c: Get rid of main_thread_id global Joel Brobecker
     [not found]   ` <83imvcg0ud.fsf@gnu.org>
2019-04-17 17:38     ` Joel Brobecker
2019-04-17 18:29       ` Eli Zaretskii
2019-04-17 22:17         ` Joel Brobecker [this message]
2019-04-18 12:52           ` Eli Zaretskii
2019-04-18 14:54             ` Joel Brobecker
2019-04-18 16:27           ` Pedro Alves
2019-04-19 20:43             ` Joel Brobecker
2019-04-22 14:24               ` Pedro Alves
2019-04-22 15:20                 ` André Pönitz
2019-04-22 17:29                   ` Pedro Alves
2019-04-22 21:42                     ` André Pönitz
2019-04-23  5:54                       ` Eli Zaretskii
2019-04-24 20:10                         ` André Pönitz
2019-04-25  5:39                           ` Eli Zaretskii
2019-04-23 11:42                       ` Pedro Alves
2019-04-28 16:58 ` [v2] Windows native GDB event handling enhancement Joel Brobecker
2019-04-28 16:58   ` [RFA v2 2/2][master only] gdb/windows-nat.c: Get rid of main_thread_id global Joel Brobecker
2019-04-28 16:58   ` [RFA v2 1/2][master+8.3] (Windows) fix thr != nullptr assert failure in delete_thread_1 Joel Brobecker
2019-04-30 13:00   ` [v2] Windows native GDB event handling enhancement Pedro Alves
2019-04-30 21:04     ` pushed(master+8.3): " Joel Brobecker

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=20190417221729.GA5839@adacore.com \
    --to=brobecker@adacore.com \
    --cc=eliz@gnu.org \
    --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