public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Phil Muldoon <pmuldoon@redhat.com>
To: Tom Tromey <tom@tromey.com>, Pedro Alves <palves@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Release the GIL while running a gdb command or expression
Date: Thu, 18 Oct 2018 09:31:00 -0000	[thread overview]
Message-ID: <72bce55f-57d8-bd77-e39a-732e85c21787@redhat.com> (raw)
In-Reply-To: <87woqhbmh6.fsf@tromey.com>

On 16/10/2018 22:39, Tom Tromey wrote:
> Pedro> Hopefully it'll be possible to tighten the test a bit to avoid that.
> 
> I should have read the test more closely.  It doesn't do what it claims
> to do at all.

I'm not sure what it or I claimed? The initial snippet and test was included
with this explanation:

"As for a test, we also have a test included. It does not appear to be
racy for our purposes. I also include it for consideration. This
snippet is just for initial consideration and will make any changes
needed to include it in the patch."

This was from the original (version 1) email.

> 
> The only way the test can fail is if one "Hello" or "Sleeping" line
> comes with the wrong number:
> 
> 	set n $expect_out(1,string)
> 	if { $hello_last + 1 != $n } {
> 	    fail $test
> 
> But this just isn't possible.

I can't remember why I wrote it this way, or if there was a
modification to the patch over the years, but, the original intention
to the patch was to test interleaved output from the Python thread and
GDB (thereby proving the GIL was released).

> 
> Also the Python code is calling print on a gdb stream in a background
> thread, which is a no-no.

Yeah, this came up in the original review thread. You pointed out that
GDB wasn't thread safe when I was worried over any existing scripts
out there that would rely on the expectation that the gdb.command API
would hold the GIL. For some reason, I did not know this beforehand
(that GDB wasn't thread safe), so that is why it snuck in there (back
in 2014).

Cheers

Phil

      parent reply	other threads:[~2018-10-18  9:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10 20:23 Tom Tromey
2018-10-12 14:52 ` Phil Muldoon
2018-10-12 16:49 ` Pedro Alves
2018-10-12 16:51   ` Pedro Alves
2018-10-16 12:48   ` Tom Tromey
2018-10-16 14:50     ` Pedro Alves
2018-10-12 16:56 ` Pedro Alves
2018-10-16 13:05   ` Tom Tromey
2018-10-16 14:51     ` Pedro Alves
2018-10-16 21:39       ` Tom Tromey
2018-10-16 21:45         ` Tom Tromey
2018-10-24 18:08           ` Pedro Alves
2018-10-25 12:46             ` Phil Muldoon
2018-11-04 15:54             ` Tom Tromey
2019-01-29 13:02               ` Tom Tromey
2019-01-29 16:37                 ` Pedro Alves
2018-10-18  9:31         ` Phil Muldoon [this message]

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=72bce55f-57d8-bd77-e39a-732e85c21787@redhat.com \
    --to=pmuldoon@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    --cc=tom@tromey.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).