* [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait()
@ 2018-08-02 16:54 Jon Turney
2018-08-07 2:45 ` Simon Marchi
0 siblings, 1 reply; 4+ messages in thread
From: Jon Turney @ 2018-08-02 16:54 UTC (permalink / raw)
To: gdb-patches; +Cc: Jon Turney
This causes the inferior to stop with SIGTTIN if it tries to read from the
terminal after it has been continued.
See https://cygwin.com/ml/cygwin/2016-09/msg00285.html for reproduction.
Since MinGW doesn't have a tcsetpgrp(), I don't think this problem would be
observed there, but Cygwin does so target_terminal::ours() will call it.
Calling target_terminal::ours() here seems to be is no longer appropriate
after the "Merge async and sync code paths" changes (as the inferior is now
in a separate process group even in sync mode(?), which is always used on
Windows targets)
This call was added in commit c44537cf (and see
https://sourceware.org/ml/gdb-patches/2007-02/msg00167.html for what it
fixed, which is not regressed by this change)
When windows_nat_target::wait() is entered, the inferior is running (either
it's been just been started or attached to, or windows_continue() was
called), so grabbing the controlling terminal away from it here seems to be
wrong, since infrun.c takes care of calling target_terminal::ours() when the
inferior stops.
gdb/ChangeLog:
2018-08-02 Jon Turney <jon.turney@dronecode.org.uk>
* windows-nat.c (windows_nat_target::wait): Remove a spurious
target_terminal::ours().
---
gdb/windows-nat.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index aea502638e..40cc224805 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1703,8 +1703,6 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
{
int pid = -1;
- target_terminal::ours ();
-
/* We loop when we get a non-standard exception rather than return
with a SPURIOUS because resume can try and step or modify things,
which needs a current_thread->h. But some of these exceptions mark
--
2.17.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait()
2018-08-02 16:54 [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait() Jon Turney
@ 2018-08-07 2:45 ` Simon Marchi
2018-08-09 13:23 ` Pedro Alves
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2018-08-07 2:45 UTC (permalink / raw)
To: Jon Turney; +Cc: gdb-patches, Pedro Alves
On 2018-08-02 12:53, Jon Turney wrote:
> This causes the inferior to stop with SIGTTIN if it tries to read from
> the
> terminal after it has been continued.
>
> See https://cygwin.com/ml/cygwin/2016-09/msg00285.html for
> reproduction.
>
> Since MinGW doesn't have a tcsetpgrp(), I don't think this problem
> would be
> observed there, but Cygwin does so target_terminal::ours() will call
> it.
>
> Calling target_terminal::ours() here seems to be is no longer
> appropriate
> after the "Merge async and sync code paths" changes (as the inferior is
> now
> in a separate process group even in sync mode(?), which is always used
> on
> Windows targets)
>
> This call was added in commit c44537cf (and see
> https://sourceware.org/ml/gdb-patches/2007-02/msg00167.html for what it
> fixed, which is not regressed by this change)
>
> When windows_nat_target::wait() is entered, the inferior is running
> (either
> it's been just been started or attached to, or windows_continue() was
> called), so grabbing the controlling terminal away from it here seems
> to be
> wrong, since infrun.c takes care of calling target_terminal::ours()
> when the
> inferior stops.
>
> gdb/ChangeLog:
>
> 2018-08-02 Jon Turney <jon.turney@dronecode.org.uk>
>
> * windows-nat.c (windows_nat_target::wait): Remove a spurious
> target_terminal::ours().
This seems good to me, but I'd rather check with Pedro, since he knows
this stuff much better.
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait()
2018-08-07 2:45 ` Simon Marchi
@ 2018-08-09 13:23 ` Pedro Alves
2018-09-23 15:24 ` Jon Turney
0 siblings, 1 reply; 4+ messages in thread
From: Pedro Alves @ 2018-08-09 13:23 UTC (permalink / raw)
To: Simon Marchi, Jon Turney; +Cc: gdb-patches
On 08/07/2018 03:45 AM, Simon Marchi wrote:
> On 2018-08-02 12:53, Jon Turney wrote:
>> This causes the inferior to stop with SIGTTIN if it tries to read from the
>> terminal after it has been continued.
>>
>> See https://cygwin.com/ml/cygwin/2016-09/msg00285.html for reproduction.
>>
>> Since MinGW doesn't have a tcsetpgrp(), I don't think this problem would be
>> observed there, but Cygwin does so target_terminal::ours() will call it.
>>
>> Calling target_terminal::ours() here seems to be is no longer appropriate
>> after the "Merge async and sync code paths" changes (as the inferior is now
>> in a separate process group even in sync mode(?), which is always used on
>> Windows targets)
I don't really understand what the async/sync code paths changes
here, but regardless ...
>>
>> This call was added in commit c44537cf (and see
>> https://sourceware.org/ml/gdb-patches/2007-02/msg00167.html for what it
>> fixed, which is not regressed by this change)
>>
>> When windows_nat_target::wait() is entered, the inferior is running (either
>> it's been just been started or attached to, or windows_continue() was
>> called), so grabbing the controlling terminal away from it here seems to be
>> wrong, since infrun.c takes care of calling target_terminal::ours() when the
>> inferior stops.
>>
>> gdb/ChangeLog:
>>
>> 2018-08-02 Jon Turney <jon.turney@dronecode.org.uk>
>>
>> Â Â Â Â * windows-nat.c (windows_nat_target::wait): Remove a spurious
>> Â Â Â Â target_terminal::ours().
>
> This seems good to me, but I'd rather check with Pedro, since he knows this stuff much better.
... this LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait()
2018-08-09 13:23 ` Pedro Alves
@ 2018-09-23 15:24 ` Jon Turney
0 siblings, 0 replies; 4+ messages in thread
From: Jon Turney @ 2018-09-23 15:24 UTC (permalink / raw)
To: gdb-patches
On 09/08/2018 14:23, Pedro Alves wrote:
> On 08/07/2018 03:45 AM, Simon Marchi wrote:
>> On 2018-08-02 12:53, Jon Turney wrote:
>>> This causes the inferior to stop with SIGTTIN if it tries to read from the
>>> terminal after it has been continued.
>>>
>>> See https://cygwin.com/ml/cygwin/2016-09/msg00285.html for reproduction.
>>>
>>> Since MinGW doesn't have a tcsetpgrp(), I don't think this problem would be
>>> observed there, but Cygwin does so target_terminal::ours() will call it.
>>>
>>> Calling target_terminal::ours() here seems to be is no longer appropriate
>>> after the "Merge async and sync code paths" changes (as the inferior is now
>>> in a separate process group even in sync mode(?), which is always used on
>>> Windows targets)
>
> I don't really understand what the async/sync code paths changes
> here, but regardless ...
Me neither. Ideally I would have noticed this regression closer to the
time those changes were made, but...
>>> This call was added in commit c44537cf (and see
>>> https://sourceware.org/ml/gdb-patches/2007-02/msg00167.html for what it
>>> fixed, which is not regressed by this change)
>>>
>>> When windows_nat_target::wait() is entered, the inferior is running (either
>>> it's been just been started or attached to, or windows_continue() was
>>> called), so grabbing the controlling terminal away from it here seems to be
>>> wrong, since infrun.c takes care of calling target_terminal::ours() when the
>>> inferior stops.
>>>
>>> gdb/ChangeLog:
>>>
>>> 2018-08-02 Jon Turney <jon.turney@dronecode.org.uk>
>>>
>>> Â Â Â Â * windows-nat.c (windows_nat_target::wait): Remove a spurious
>>> Â Â Â Â target_terminal::ours().
>>
>> This seems good to me, but I'd rather check with Pedro, since he knows this stuff much better.
>
> ... this LGTM.
Thanks.
0c0a40e0ab..a44294f5ed master -> master
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-23 15:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-02 16:54 [PATCH] Remove a spurious target_terminal::ours() from windows_nat_target::wait() Jon Turney
2018-08-07 2:45 ` Simon Marchi
2018-08-09 13:23 ` Pedro Alves
2018-09-23 15:24 ` Jon Turney
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).