ValhallaTeam / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

Crash in eglReleaseThread #439

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. use the following sequence of code to shut down EGL:

    eglMakeCurrent(eglGlobalDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
    eglDestroySurface(eglGlobalDisplay, eglGlobalSurface);
    eglDestroyContext(eglGlobalDisplay, eglGlobalContext);
    eglTerminate(eglGlobalDisplay);
    eglReleaseThread();

What is the expected output? What do you see instead?
I expect the program to exit. Instead it crashes.

What version of the product are you using? On what operating system?
dx11-proto branch, Windows 7.

Please provide any additional information below.
It crashes in eglMakeCurrent at the following line

            rx::Renderer *renderer = display->getRenderer();

when eglReleaseThread makes the following call to it:

        eglMakeCurrent(EGL_NO_DISPLAY, EGL_NO_CONTEXT, EGL_NO_SURFACE, EGL_NO_SURFACE);

It crashes because, er.., display == NULL. So actually the order of shutdown 
doesn't matter. I guess none of your other users has ever called 
eglReleaseThread.

Original issue reported on code.google.com by callow.m...@artspark.co.jp on 19 Jun 2013 at 10:15

GoogleCodeExporter commented 9 years ago
Here is a proposed patch for this crash. It looks pretty safe as glMakeCurrent, 
which will still be calledd by eglMakeCurrent, does nothing unless display, 
context & surface are all non-zero. Our app no longer crashes with this change.

Original comment by callow.m...@artspark.co.jp on 20 Jun 2013 at 7:38

Attachments:

GoogleCodeExporter commented 9 years ago
It looks like this would have provoked a crash even before the move to dx11. 
Thank you for providing a patch. Our preferred process for contributions is to 
use the review tool as described on the wiki, as it makes the review process 
easier: https://code.google.com/p/angleproject/wiki/ContributingCode

(Also, it looks as though you may have used tabs instead of spaces in the 
patch.)

Original comment by shannonw...@chromium.org on 24 Jun 2013 at 5:38

GoogleCodeExporter commented 9 years ago
Apologies for the tabs. I thought I'd removed them.

I'll have to read ContributingCode again. Last time I looked at it, it just 
covered the legal nonsense.

Original comment by callow.m...@artspark.co.jp on 28 Jun 2013 at 3:31

GoogleCodeExporter commented 9 years ago
When will you integrate this, or a better, fix into ANGLE? This crash happens 
any time eglReleaseThread is called. The only way to avoid it is to not call 
the function. Perhaps eglReleaseThread is not strictly necessary with ANGLE but 
portable code is likely to use it anyway.

Original comment by callow.m...@artspark.co.jp on 17 Jul 2013 at 8:14

GoogleCodeExporter commented 9 years ago
I'm sorry, Mark. Our attention has been focused on the ongoing development on 
the es3proto branch. This patch looks fine to me, aside from the tabs. I'll see 
about landing it soon.

Using the process on the ContributingCode page, and uploading patches to the 
review tool described there, is the best way to make sure proposed patches stay 
on our radar.

Original comment by shannonw...@chromium.org on 17 Jul 2013 at 4:30

GoogleCodeExporter commented 9 years ago

Original comment by shannonw...@chromium.org on 18 Jul 2013 at 2:26

GoogleCodeExporter commented 9 years ago
Landed in master: rba6944849acb
Landed in legacy: r74fe3199d7c6
Landed in es3proto: rac4109a39d77

Original comment by shannonw...@chromium.org on 24 Jul 2013 at 11:15