macOS: Fix a deadlock with CVDisplayLink

I had added an optimization to not pass messages to
main thread every time the CVDisplayLink timer fired, unless
a render frame for that monitor was actually requested.

However, this optimization is impossible to implement wihtout a deadlock
since CVDisplayLink has its own internal lock that it does not expose.

So I guess macOS users with multiple monitors will simply have to take
the performance hit of useless wakeups sixty times a second for every
extra monitor.

Fixes #1779
This commit is contained in:
Kovid Goyal 2019-07-05 20:16:32 +05:30
parent edc8878632
commit 39f6071c68
No known key found for this signature in database
GPG Key ID: 06BC317B515ACE7C
5 changed files with 16 additions and 35 deletions

View File

@ -41,6 +41,8 @@ To update |kitty|, :doc:`follow the instructions <binary>`.
- Linux: Use the system "bell" sound for the terminal bell. Adds libcanberra - Linux: Use the system "bell" sound for the terminal bell. Adds libcanberra
as a new dependency to play the system sound. as a new dependency to play the system sound.
- macOS: Fix a rare deadlock causing kitty to hang (:iss:`1779`)
0.14.2 [2019-06-09] 0.14.2 [2019-06-09]
--------------------- ---------------------

View File

@ -399,7 +399,6 @@ int _glfwPlatformInit(void)
if (!initializeTIS()) if (!initializeTIS())
return false; return false;
_glfw.ns.displayLinks.lock = [NSLock new];
_glfwInitTimerNS(); _glfwInitTimerNS();
_glfwInitJoysticksNS(); _glfwInitJoysticksNS();
@ -413,11 +412,7 @@ void _glfwPlatformTerminate(void)
{ {
@autoreleasepool { @autoreleasepool {
if (_glfw.ns.displayLinks.lock) {
_glfwClearDisplayLinks(); _glfwClearDisplayLinks();
[_glfw.ns.displayLinks.lock release];
_glfw.ns.displayLinks.lock = nil;
}
if (_glfw.ns.inputSource) if (_glfw.ns.inputSource)
{ {

View File

@ -243,16 +243,15 @@ bool refreshMonitorScreen(_GLFWmonitor* monitor)
////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////
void _glfwClearDisplayLinks() { void _glfwClearDisplayLinks() {
[_glfw.ns.displayLinks.lock lock];
for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) { for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) {
if (_glfw.ns.displayLinks.entries[i].displayLink) { if (_glfw.ns.displayLinks.entries[i].displayLink) {
CVDisplayLinkStop(_glfw.ns.displayLinks.entries[i].displayLink); CVDisplayLinkStop(_glfw.ns.displayLinks.entries[i].displayLink);
CVDisplayLinkRelease(_glfw.ns.displayLinks.entries[i].displayLink); CVDisplayLinkRelease(_glfw.ns.displayLinks.entries[i].displayLink);
_glfw.ns.displayLinks.entries[i].displayLink = nil; _glfw.ns.displayLinks.entries[i].displayLink = nil;
_glfw.ns.displayLinks.entries[i].lastRenderFrameRequestedAt = 0;
} }
} }
_glfw.ns.displayLinks.count = 0; _glfw.ns.displayLinks.count = 0;
[_glfw.ns.displayLinks.lock unlock];
} }
static CVReturn displayLinkCallback( static CVReturn displayLinkCallback(
@ -261,28 +260,13 @@ static CVReturn displayLinkCallback(
CVOptionFlags flagsIn UNUSED, CVOptionFlags* flagsOut UNUSED, void* userInfo) CVOptionFlags flagsIn UNUSED, CVOptionFlags* flagsOut UNUSED, void* userInfo)
{ {
CGDirectDisplayID displayID = (CGDirectDisplayID)userInfo; CGDirectDisplayID displayID = (CGDirectDisplayID)userInfo;
[_glfw.ns.displayLinks.lock lock];
bool notify = false;
for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) {
if (_glfw.ns.displayLinks.entries[i].displayID == displayID) {
if (_glfw.ns.displayLinks.entries[i].renderFrameRequested) {
notify = true;
_glfw.ns.displayLinks.entries[i].renderFrameRequested = false;
}
break;
}
}
[_glfw.ns.displayLinks.lock unlock];
if (notify) {
NSNumber *arg = [NSNumber numberWithUnsignedInt:displayID]; NSNumber *arg = [NSNumber numberWithUnsignedInt:displayID];
[NSApp performSelectorOnMainThread:@selector(render_frame_received:) withObject:arg waitUntilDone:NO]; [NSApp performSelectorOnMainThread:@selector(render_frame_received:) withObject:arg waitUntilDone:NO];
[arg release]; [arg release];
}
return kCVReturnSuccess; return kCVReturnSuccess;
} }
static inline void createDisplayLink(CGDirectDisplayID displayID) { static inline void createDisplayLink(CGDirectDisplayID displayID) {
[_glfw.ns.displayLinks.lock lock];
if (_glfw.ns.displayLinks.count >= sizeof(_glfw.ns.displayLinks.entries)/sizeof(_glfw.ns.displayLinks.entries[0]) - 1) return; if (_glfw.ns.displayLinks.count >= sizeof(_glfw.ns.displayLinks.entries)/sizeof(_glfw.ns.displayLinks.entries[0]) - 1) return;
for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) { for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) {
if (_glfw.ns.displayLinks.entries[i].displayID == displayID) return; if (_glfw.ns.displayLinks.entries[i].displayID == displayID) return;
@ -292,7 +276,6 @@ static inline void createDisplayLink(CGDirectDisplayID displayID) {
entry->displayID = displayID; entry->displayID = displayID;
CVDisplayLinkCreateWithCGDisplay(displayID, &entry->displayLink); CVDisplayLinkCreateWithCGDisplay(displayID, &entry->displayLink);
CVDisplayLinkSetOutputCallback(entry->displayLink, &displayLinkCallback, (void*)(uintptr_t)displayID); CVDisplayLinkSetOutputCallback(entry->displayLink, &displayLinkCallback, (void*)(uintptr_t)displayID);
[_glfw.ns.displayLinks.lock unlock];
} }
// Poll for changes in the set of connected monitors // Poll for changes in the set of connected monitors

View File

@ -144,7 +144,7 @@ typedef struct _GLFWDisplayLinkNS
{ {
CVDisplayLinkRef displayLink; CVDisplayLinkRef displayLink;
CGDirectDisplayID displayID; CGDirectDisplayID displayID;
bool renderFrameRequested; double lastRenderFrameRequestedAt;
} _GLFWDisplayLinkNS; } _GLFWDisplayLinkNS;
// Cocoa-specific global data // Cocoa-specific global data
@ -183,7 +183,6 @@ typedef struct _GLFWlibraryNS
struct { struct {
_GLFWDisplayLinkNS entries[256]; _GLFWDisplayLinkNS entries[256];
size_t count; size_t count;
id lock;
} displayLinks; } displayLinks;
} _GLFWlibraryNS; } _GLFWlibraryNS;

View File

@ -67,13 +67,12 @@ static unsigned long long display_link_shutdown_timer = 0;
void void
_glfwShutdownCVDisplayLink(unsigned long long timer_id UNUSED, void *user_data UNUSED) { _glfwShutdownCVDisplayLink(unsigned long long timer_id UNUSED, void *user_data UNUSED) {
[_glfw.ns.displayLinks.lock lock];
display_link_shutdown_timer = 0; display_link_shutdown_timer = 0;
for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) { for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) {
_GLFWDisplayLinkNS *dl = &_glfw.ns.displayLinks.entries[i]; _GLFWDisplayLinkNS *dl = &_glfw.ns.displayLinks.entries[i];
if (dl->displayLink) CVDisplayLinkStop(dl->displayLink); if (dl->displayLink) CVDisplayLinkStop(dl->displayLink);
dl->lastRenderFrameRequestedAt = 0;
} }
[_glfw.ns.displayLinks.lock unlock];
} }
static inline void static inline void
@ -86,23 +85,26 @@ requestRenderFrame(_GLFWwindow *w, GLFWcocoarenderframefun callback) {
w->ns.renderFrameCallback = callback; w->ns.renderFrameCallback = callback;
w->ns.renderFrameRequested = true; w->ns.renderFrameRequested = true;
CGDirectDisplayID displayID = displayIDForWindow(w); CGDirectDisplayID displayID = displayIDForWindow(w);
[_glfw.ns.displayLinks.lock lock];
if (display_link_shutdown_timer) { if (display_link_shutdown_timer) {
_glfwPlatformUpdateTimer(display_link_shutdown_timer, DISPLAY_LINK_SHUTDOWN_CHECK_INTERVAL, true); _glfwPlatformUpdateTimer(display_link_shutdown_timer, DISPLAY_LINK_SHUTDOWN_CHECK_INTERVAL, true);
} else { } else {
display_link_shutdown_timer = _glfwPlatformAddTimer(DISPLAY_LINK_SHUTDOWN_CHECK_INTERVAL, false, _glfwShutdownCVDisplayLink, NULL, NULL); display_link_shutdown_timer = _glfwPlatformAddTimer(DISPLAY_LINK_SHUTDOWN_CHECK_INTERVAL, false, _glfwShutdownCVDisplayLink, NULL, NULL);
} }
double now = glfwGetTime();
for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) { for (size_t i = 0; i < _glfw.ns.displayLinks.count; i++) {
_GLFWDisplayLinkNS *dl = &_glfw.ns.displayLinks.entries[i]; _GLFWDisplayLinkNS *dl = &_glfw.ns.displayLinks.entries[i];
if (dl->displayID == displayID) { if (dl->displayID == displayID) {
dl->renderFrameRequested = true; dl->lastRenderFrameRequestedAt = now;
if (!CVDisplayLinkIsRunning(dl->displayLink)) { if (!CVDisplayLinkIsRunning(dl->displayLink)) {
CVDisplayLinkStart(dl->displayLink); CVDisplayLinkStart(dl->displayLink);
} } else {
break; if (dl->lastRenderFrameRequestedAt && now - dl->lastRenderFrameRequestedAt) {
if (dl->displayLink) CVDisplayLinkStop(dl->displayLink);
dl->lastRenderFrameRequestedAt = 0;
}
}
} }
} }
[_glfw.ns.displayLinks.lock unlock];
} }
void void