[Mesa-dev] [PATCH 12/17] i965: Replace exit(1) with abort() when command submission fails.

[Eero Tamminen]

Hi,

On 06.09.2017 03:09, Kenneth Graunke wrote:
> Calling exit(1) when execbuffer fails is not necessarily safe.
> When running Piglit tests with a Mesa that submitted invalid commands
> to the GPU, I discovered the following problem:
> 
> 1. do_flush_locked fails and calls exit(1)...invoking atexit handlers.
> 2. Piglit tries to clean up after itself, and does eglMakeCurrent to
>     release the current context.
> 3. MakeCurrent calls glFlush (or the internal hook for that)
> 4. glFlush calls intel_batchbuffer_flush
> 
> So we end up trying to flush the batch...in the middle of flushing the
> batch...with code that isn't designed to be reentrant.  So it breaks
> even worse than before.  In my case, it just outright crashed.
> 
> Calling abort() is probably not what we want either, but it at least
> bypasses atexit() handlers, so it won't hit this ugly reentrancy
> problem.

If you just want to avoid atexit() handlers, you can use _exit().

That one doesn't trigger ABORT handler.

(Which may be good or bad depending on what you want.  I think SDL by 
default traps ABORT among many other things, to restore screen mode on 
emergency exits.)


	- Eero

> ---
>   src/mesa/drivers/dri/i965/intel_batchbuffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 491aa12dd63..637705226f0 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -679,7 +679,7 @@ do_flush_locked(struct brw_context *brw, int in_fence_fd, int *out_fence_fd)
>   
>      if (ret != 0) {
>         fprintf(stderr, "intel_do_flush_locked failed: %s\n", strerror(-ret));
> -      exit(1);
> +      abort();
>      }
>   
>      return ret;
>