Re: call_function_by_hand doesn't restore target async?

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 04 December 2008 20:18:37, Doug Evans wrote:

> Should there be a cleanup to restore target_async_mask?
> 

>     if (target_can_async_p ())
>       saved_async = target_async_mask (0);
> 
>     old_cleanups2 = make_cleanup_restore_integer (&suppress_resume_observer);
>     suppress_resume_observer = 1;
>     make_cleanup_restore_integer (&suppress_stop_observer);
>     suppress_stop_observer = 1;
>     proceed (real_pc, TARGET_SIGNAL_0, 0);
>     do_cleanups (old_cleanups2);
> 
>     if (saved_async)
>       target_async_mask (saved_async);

> 
> target.h has this:
> 
> /* This is to be used ONLY within call_function_by_hand(). It provides
>    a workaround, to have inferior function calls done in sychronous
>    mode, even though the target is asynchronous. After
>    target_async_mask(0) is called, calls to target_can_async_p() will
>    return FALSE , so that target_resume() will not try to start the
>    target asynchronously. After the inferior stops, we IMMEDIATELY
>    restore the previous nature of the target, by calling
>    target_async_mask(1). After that, target_can_async_p() will return
>    TRUE. ANY OTHER USE OF THIS FEATURE IS DEPRECATED.

That's the idealistic theory anyway...  It's also used, although not
through the target vector, in linux_nat_create_inferior.  In practice, and
especially with non-stop mode, making infcalls async is ranging somewhere
from hard to impossible.  An alternative path I've considered to remove
this masking, is to add an `options' parameter to target_wait so we'd pass
a 'TARGET_WNOHANG' flag to it when you want asyncness (in fetch_inferior_event),
and pass `0' in the call in wait_for_inferior (that's always blocking).
target_wait is modelled on `wait(pid)', so it sounds a good fit to me.
I've actually implemented it that way in gdbserver in the
multiprocess branch.

> I don't see any other calls to target_async_mask.  Given that it's
> only to be used by call_function_by_hand that's understandable,
> but then I don't understand how target_async_mask gets restored
> if proceed errors out.

Right, it doesn't.  inf-loop.c:inferior_event_handler has a
drastic attitude about exceptions --- it always pops the target, which
means that a cleanup will would most of the times set the async mask
in the wrong target, and thus say, e.g., remote_async_mask_value
will still be left dangling...  I think that adjusting the
target_wait interface like described above would be the best way
to fix this.

> I'll add a fix to my dummy-frames patch if there's a bug here.

> If there isn't a bug I'll at least add a comment. :-)
> [and I'll need to start breaking it into a set of smaller patches ...]
> 

-- 
Pedro Alves