Re: [RFA/commit] GDB crash re-running program on Windows (native)

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-01-02 22:33, Joel Brobecker wrote:
> Hi Simon,
> 
> Thanks for the review!
> 
>> > With that in mind, this commit fixes the issue by deleting the thread
>> > when the inferior sends the exit-process event as opposed to deleting it
>> > later, while starting a new inferior.
>> >
>> > This restores also restores the printing of the thread-exit notification
>> 
>> "This restores also restores"
>> 
>> > for the main thread, which was missing before. Looking at the transcript
>> > of the example show above, we can see 4 thread creation notifications,
>> > and only 3 notifications for thread exits. Now creation and exit
>> > notifications are balanced.
>> 
>> Another choice is to not show the main thread's creation and exit (as 
>> is
>> done on Linux), since it's kind of redundant with the process creation 
>> and
>> exit.
> 
> Indeed, that's a good idea. I propose to work on that as a followup
> patch, as I am a little short on time these next couple of weeks
> (broken hand :-().
> 
>> > In the handling of EXIT_THREAD_DEBUG_EVENT, the main_thread_id
>> > check is removed because deemed unnecessary: The main thread was
>> > introduced by a CREATE_THREAD_DEBUG_EVENT, and thus the kernel
>> > is expected to report its death via EXIT_PROCESS_DEBUG_EVENT.
>> 
>> Should that last EXIT_PROCESS_DEBUG_EVENT actually be
>> EXIT_THREAD_DEBUG_EVENT?
> 
> Not quite, but actually a good question nonetheless. There was
> an auto-completion error in the text above. New text:
> 
>     | In the handling of EXIT_THREAD_DEBUG_EVENT, the main_thread_id
>     | check is removed because deemed unnecessary: The main thread was
>     | introduced by a CREATE_PROCESS_DEBUG_EVENT, and thus the kernel
>     | is expected to report its death via EXIT_PROCESS_DEBUG_EVENT.
> 
> In other words, we don't expect to receive an EXIT_THREAD_DEBUG_EVENT
> for the main thread, because, at the Windows level, that thread really
> isn't a thread, but really a process.
> 
> 
>> > @@ -1607,6 +1599,9 @@ get_windows_debug_event (struct target_ops *ops,
>> >  	}
>> >        else if (saw_create == 1)
>> >  	{
>> > +	  windows_delete_thread (ptid_t (current_event.dwProcessId, 0,
>> > +					 main_thread_id),
>> > +				 0);
>> >  	  ourstatus->kind = TARGET_WAITKIND_EXITED;
>> >  	  ourstatus->value.integer = current_event.u.ExitProcess.dwExitCode;
>> >  	  thread_id = main_thread_id;
>> 
>> If what you said above is right (that the kernel reports the main 
>> thread's
>> death through an EXIT_THREAD_DEBUG_EVENT), why is this new call to
>> windows_delete_thread needed?  Shouldn't it already be deleted at this
>> point?
> 
> Does the answer above allow us to make sense of this hunk?

Yes, thanks for the explanation, everything LGTM.

Simon