On Saturday 28 June 2008 07:51:53 am Massimo Del Fedele wrote:
Roderick Colenbrander ha scritto:
Hi,
I don't think we want to go this way. First of all we want to emulate win32 opengl. If windows does this we MIGHT have to do something like this.
Well, I've never seen windows hang for an opengl call exception... But I can be wrong. IMHO the exception should be catched and/or propagated, not allowed to block the whole app..... Here the problem is that an exception thrown by gl code skips the wine_tsx11_unlock() call, blocking all following X11 code.
I think the issue is that it shouldn't be crashing. Especially if we don't know why it's crashing, this patch is just hiding the error more than anything. For all we know it can be a buffer overrun somewhere, and you don't want to just work around things like that.
Second all opengl calls hit ENTER_GL / LEAVE_GL and these few extra calls affect performance.
which extra calls ?
The __TRY stuff. It's rather expensive to set up an exception block, even in C++ where it's part of the language (let alone how Wine does it in C). But we do know D3D doesn't catch exceptions in most cases (it can crash in plenty of ways), so I don't think this is correct anyway.
Chris Robinson ha scritto:
On Saturday 28 June 2008 07:51:53 am Massimo Del Fedele wrote:
Roderick Colenbrander ha scritto:
Hi,
I don't think we want to go this way. First of all we want to emulate win32 opengl. If windows does this we MIGHT have to do something like this.
Well, I've never seen windows hang for an opengl call exception... But I can be wrong. IMHO the exception should be catched and/or propagated, not allowed to block the whole app..... Here the problem is that an exception thrown by gl code skips the wine_tsx11_unlock() call, blocking all following X11 code.
I think the issue is that it shouldn't be crashing. Especially if we don't know why it's crashing,...
well, there are not so many places... it crashes on a glGetString(), which accepts a scalar value and returns a static string pointer. No dynamic data, no bad pointers outside it. So it must be a crash INSIDE opengl code. It's called (wrongly) without an opengl context, but that's how autocad installer does, so it should NOT crash because of that. In windows it just returns NULL pointer, in wine, with a simple test app it also returns NULL, so we can't reproduce with a simple test case. Maybe some previous GL call damages some internal opengl data... but I'm still convinced that as it doesn't crash in windows it should not do it in wine. Or, at least it should free the lock when crashing.....
this patch is just hiding the error more than anything.
Not exactly. Latest patch (the try...finally) doesn't hide anything, it just frees the lock upon crash in gl code, which imho is correct behaviour. Autocad installer ignores the crash and follows, with the patch. Without it, it hangs on next x11 call.
For all we know it can be a buffer overrun somewhere, and you don't
want to just work around things like that.
Second all opengl calls hit ENTER_GL / LEAVE_GL and these few extra calls affect performance.
which extra calls ?
The __TRY stuff. It's rather expensive to set up an exception block, even in C++ where it's part of the language (let alone how Wine does it in C). But we do know D3D doesn't catch exceptions in most cases (it can crash in plenty of ways), so I don't think this is correct anyway.
The first patch proposed (and another, more complete), IS a workaround. We checks if a context exists and, if none, we just return NULL instead locking-calling-unlocking. Windows behaviour is that one, so it should do no harm.
The problem here is not catching/not catching the exception; I agree that it should be uncaught and let to user code. The problem is the stale lock with has nothing to do with user code. If some app has a code like that :
try { do_some_gl_call(); } catch (...) error_handling() } some_x11_call(); <== it will hang here
On ANY opengl exception it'll not have any chance to correct the error and continue.
Max
On Saturday 28 June 2008 01:50:09 pm Massimo Del Fedele wrote:
Chris Robinson ha scritto:
I think the issue is that it shouldn't be crashing. Especially if we don't know why it's crashing,...
well, there are not so many places... it crashes on a glGetString(), which accepts a scalar value and returns a static string pointer. No dynamic data, no bad pointers outside it. So it must be a crash INSIDE opengl code. It's called (wrongly) without an opengl context, but that's how autocad installer does, so it should NOT crash because of that.
And as was shown, Wine correctly returns NULL when its called without a context, so there is something else making it crash. It may be a buggy driver. It may be memory corruption Wine is causing.. who knows.
but I'm still convinced that as it doesn't crash in windows it should not do it in wine.
Of course, which is why we should try to find the reason it crashes and fix it, not hide the crash behind a costly try block.
-
Chris Robinson -
Massimo Del Fedele