Re: [PATCH] [Server]: when getting the status of an exception, ensure the debugger continue information is not erased when releasing objects

[Alexandre Julliard]

Eric Pouech <eric.pouech(a)orange.fr> writes:
> diff --git a/server/debugger.c b/server/debugger.c
> index 795a24a..4458a3b 100644
> --- a/server/debugger.c
> +++ b/server/debugger.c
> @@ -677,6 +677,8 @@ DECL_HANDLER(get_exception_status)
>      if ((event = (struct debug_event *)get_handle_obj( current->process, req->handle,
>                                                         0, &debug_event_ops )))
>      {
> +        NTSTATUS status = STATUS_PENDING;
> +
>          close_handle( current->process, req->handle );
>          if (event->state == EVENT_CONTINUED)
>          {
> @@ -686,10 +688,10 @@ DECL_HANDLER(get_exception_status)
>                  set_reply_data( &event->context, size );
>                  current->context = NULL;
>              }
> -            set_error( event->status );
> +            status = event->status;
>          }
> -        else set_error( STATUS_PENDING );
>          release_object( event );
> +        set_error( status );

Releasing an object should never change the error code. Where do you see
that happening?

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH] [Server]: when getting the status of an exception, ensure the debugger continue information is not erased when releasing objects

[Eric Pouech]

Alexandre Julliard a écrit :
> Eric Pouech <eric.pouech(a)orange.fr> writes:
> > diff --git a/server/debugger.c b/server/debugger.c
> > index 795a24a..4458a3b 100644
> > --- a/server/debugger.c
> > +++ b/server/debugger.c
> > @@ -677,6 +677,8 @@ DECL_HANDLER(get_exception_status)
> >      if ((event = (struct debug_event *)get_handle_obj( current->process, req->handle,
> >                                                         0, &debug_event_ops )))
> >      {
> > +        NTSTATUS status = STATUS_PENDING;
> > +
> >          close_handle( current->process, req->handle );
> >          if (event->state == EVENT_CONTINUED)
> >          {
> > @@ -686,10 +688,10 @@ DECL_HANDLER(get_exception_status)
> >                  set_reply_data( &event->context, size );
> >                  current->context = NULL;
> >              }
> > -            set_error( event->status );
> > +            status = event->status;
> >          }
> > -        else set_error( STATUS_PENDING );
> >          release_object( event );
> > +        set_error( status );
>
> Releasing an object should never change the error code. Where do you see
> that happening?

what happens is something like:
process P being debugged by D
- while processing a debug event (fetched in P, P calls 
ntdll(a)send_debug_event, and waits for the debugger to process the event)
- D gets the event, and the user asks for quit in D. D sends back to 
wineserver a DBG_CONTINUE response to the event
- D then dies before P runs again
- P then calls server(a)get_exception_status, where when getting the 
status from the debugger, (which should be DBG_CONTINUE, and it is 
before releasing the debug event). but while releasing the debug event, 
someone clears the status, and we get 0 from the get_exception_status event

I didn't check in details which clear_error() is to blame in server, but 
close_thread_desktop (from a cursory look) could be to blame

A+

-- 
Eric Pouech
"The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

[PATCH] [Server]: when getting the status of an exception, ensure the debugger continue information is not erased when releasing objects

[Alexandre Julliard]

Eric Pouech <eric.pouech(a)orange.fr> writes:
> what happens is something like:
> process P being debugged by D
> - while processing a debug event (fetched in P, P calls
> ntdll(a)send_debug_event, and waits for the debugger to process the
> event)
> - D gets the event, and the user asks for quit in D. D sends back to
> wineserver a DBG_CONTINUE response to the event
> - D then dies before P runs again
> - P then calls server(a)get_exception_status, where when getting the
> status from the debugger, (which should be DBG_CONTINUE, and it is
> before releasing the debug event). but while releasing the debug
> event, someone clears the status, and we get 0 from the
> get_exception_status event
>
> I didn't check in details which clear_error() is to blame in server,
> but close_thread_desktop (from a cursory look) could be to blame

That one probably shouldn't clear the error, but I don't see how you'd
get there by releasing an event. Do you have a backtrace?

-- 
Alexandre Julliard
julliard(a)winehq.org