Re: [patchv2 2/2] Fix CTRL-C for remote.c (PR remote/15297)

Re: [patchv2 2/2] Fix CTRL-C for remote.c (PR remote/15297)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Primarily this patch removes some heavy functions from the signal
Jan> handlers as signal handlers can call only few signal-safe functions
Jan> according to POSIX.  Currently with "set debug remote 1" CTRL-C
Jan> typically locks up on malloc where SIGINT handler interrupted also
Jan> malloc.

I read through this patch and it made sense, at least as far as I could
understand it.  It's a tricky area and without redoing the research you
did I think it is reasonably hard to critique.

Jan> The testcase does not work perfectly for target-async + all-stop,
Jan> it is not being tested.  I did not find it a commonly used mode and
Jan> it may be fixed in a different/additional patch.  It works in
Jan> general but not in 100% cases.

Can you characterize the failure modes?  Or will I see it if I add a
target-async+non-stop case to your new .exp?

I have a series here to always enable target-async, which I plan to
submit soon.  I'd like to understand this more so I can fix up my
series.

Tom

Re: [patchv2 2/2] Fix CTRL-C for remote.c (PR remote/15297)

[Jan Kratochvil]

On Thu, 25 Jul 2013 17:19:57 +0200, Tom Tromey wrote:
> I read through this patch and it made sense, at least as far as I could
> understand it.  It's a tricky area and without redoing the research you
> did I think it is reasonably hard to critique.

Thanks.


> Jan> The testcase does not work perfectly for target-async + all-stop,
> Jan> it is not being tested.  I did not find it a commonly used mode and
> Jan> it may be fixed in a different/additional patch.  It works in
> Jan> general but not in 100% cases.
> 
> Can you characterize the failure modes?  Or will I see it if I add a
> target-async+non-stop case to your new .exp?
  ^^^^^^^^^^^^^^^^^^^^^ - this one is there:

There are currently:
	async-nonstop
	sync-allstop

There is missing (and causing FAILs if added):
	async-allstop


> I have a series here to always enable target-async, which I plan to
> submit soon.  I'd like to understand this more so I can fix up my
> series.

You are right this "async-allstop" case has to be added.  I was not sure what
is the expected behavior/output in such case, maybe GDB behaves correctly.


The patch got out of sync anyway, I will resubmit it.


Thanks,
Jan

Re: [patchv2 2/2] Fix CTRL-C for remote.c (PR remote/15297)

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

>> Can you characterize the failure modes?  Or will I see it if I add a
>> target-async+non-stop case to your new .exp?
Jan>   ^^^^^^^^^^^^^^^^^^^^^ - this one is there:

Sorry, I meant target-async+full-stop.

Jan> You are right this "async-allstop" case has to be added.  I was not
Jan> sure what is the expected behavior/output in such case, maybe GDB
Jan> behaves correctly.

Ok, fair enough.  I will look after it goes in.

Tom