Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use serial queues and revert string copy changes #156

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

edusperoni
Copy link
Collaborator

No description provided.

@cla-bot cla-bot bot added the cla: yes label Mar 24, 2022
@rigor789 rigor789 changed the title fix: use serail queues and revert string copy changes fix: use serial queues and revert string copy changes Mar 24, 2022
@NathanWalker NathanWalker merged commit e8681ff into main Mar 30, 2022
@NathanWalker NathanWalker deleted the fix/sync-message-queue branch March 30, 2022 15:58
Comment on lines +173 to +180
dispatch_sync(this->messageLoopQueue_, ^{
loopsRunning = runningNestedLoops_;
terminated_ = false;
if (runningNestedLoops_) {
return;
}
this->runningNestedLoops_ = true;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @edusperoni, curious what issue/scenario led to the introduction of these additional dispatch_sync wrappers in runMessageLoopOnPause and quitMessageLoopOnPause. I identified the culprit messageLoopQueue_ after a while today trying to work out why my app would crash every time I triggered a breakpoint or debugger statement in my code by executing the containing function from devtools console.

When we have a debugger statement or breakpoint set somewhere in our code, say in a function defined in global scope, and we trigger this by evaluating an expression in DevTools console e.g. calling the function, this now creates a deadlock. When the debugger statement gets hit in the same synchronous execution initiated by this->dispatchMessage(message) in onFrontendMessageReceived, we end up in runMessageLoopOnPause, which now schedules its block to check for runningNestedLoops_ into messageLoopQueue_ - but that queue is currently waiting for the execution of runMessageLoopOnPause to finish.

My idea was that there should be a separate queue for execution/dispatch of messages from the one(s) in which blocks pump new messages, which blocks would be added to via dispatch_async, but I hit various issues trying to implement it that way e.g. dispatch_semaphore_wait never returning. I have ended up just commenting all the dispatch_sync(this->messageLoopQueue_ wrappers which seems to have fixed the crash I was having issues with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants