We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4932 - NSOpenGLContext setView and update methods must be called from main thread
Summary: NSOpenGLContext setView and update methods must be called from main thread
Status: REOPENED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 2.0.15
Hardware: All macOS 10.15
: P2 critical
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.14
Depends on:
Blocks:
 
Reported: 2020-01-08 00:27 UTC by Tim McDaniel
Modified: 2021-02-08 18:04 UTC (History)
2 users (show)

See Also:

Attachments
dispatch GL context updates to main thread (2.96 KB, patch)
2020-01-08 20:31 UTC, Tim McDaniel 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim McDaniel 2020-01-08 00:27:46 UTC 
Similar to bug 4714, but new as of macOS 10.15 when building with Mac SDK 10.15.

Calling NSOpenGLContext 'setView' or 'update' methods on a non-main thread will cause a hard crash (illegal instruction exception), so these methods must be called from the main thread.  The simple way to accomplish this is:

if ([NSThread isMainThread]) {
    [super update]; // assuming super is an NSOpenGLContext
} else {
    dispatch_sync(dispatch_get_main_queue(), ^{ [super update]; });
}

Note that the isMainThread check is necessary, since dispatch_sync to the main queue cannot be called on the main thread, because the main queue is already on the main thread, and this would cause deadlock.

This fix should be applied to all setView and update calls in SDL_cocoaopengl.m.

Similar fixes can be found in several projects online, such as:
https://github.com/google/filament/pull/1959/commits/5d0bc0d5416e0da91ca06575fb1838ac01a97143

In bug 4714, some concern was expressed about the performance implications of dispatch_sync, particularly if SDL_GL_MakeCurrent might be called every frame.  This was perhaps a valid concern, but considering the alternative is now a crash, well...
Comment 1 Tim McDaniel 2020-01-08 00:50:39 UTC 
I should add: In our internal SDL fork, in Cocoa_GL_MakeCurrent, as an optimization we skip

[nscontext setWindow:window];
[nscontext updateIfNeeded];

if the passed in window is the same as the nscontext's current window, since those calls are unnecessary in that case.
Comment 2 Sam Lantinga 2020-01-08 15:18:36 UTC 
Thanks, can you provide a tested patch?
Comment 3 Tim McDaniel 2020-01-08 20:31:59 UTC 
Created attachment 4155 [details]
dispatch GL context updates to main thread

I can't vouch for the viability of this patch, as I have not tested it as-is.  I have removed some of our mods that would only muddy the waters, and are specific to our SDL fork.  Specifically, with our mods, we take explicit control of GL context updates by passing in a NULL window pointer to SDL_GL_MakeCurrent, which disables SDL's automatic context update mechanism (e.g. context update when window is resized) and causes an explicit forced update.
Comment 4 Ryan C. Gordon 2020-04-07 18:29:38 UTC 
This patch looks reasonable to me.

With minor changes to make it apply, it's now https://hg.libsdl.org/SDL/rev/19c4516de68d, thanks!

--ryan.
Comment 5 Thomas Altenburger 2021-02-08 18:04:16 UTC 
Hello there,

Maintainer of MonoGame here. MonoGame uses SDL on macOS and we believe that this issue might not have been addressed properly.

We have a macOS build bot on the MonoGame repository which runs tests in threads with NUnit and since SDL 2.0.14 the tests are getting frozen upon calling SDL_GL_CreateContext (which never returns).

We assumed a threading issue and narrowed this down to these changes https://hg.libsdl.org/SDL/rev/19c4516de68d

Reverting them worked.

Cheers,
Thomas