Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[default] Fix possible crash at RunLoop::wake() #16255

Merged
merged 2 commits into from
Mar 2, 2020

Conversation

pozdnyakov
Copy link
Contributor

The runOnce() call during run loop destruction might cause invoke() calls.

Fixes #16250

@pozdnyakov pozdnyakov self-assigned this Mar 1, 2020
Copy link
Contributor

@alexshalamov alexshalamov left a comment

Choose a reason for hiding this comment

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

lgtm

@pozdnyakov pozdnyakov merged commit 8c0f90c into master Mar 2, 2020
@pozdnyakov pozdnyakov deleted the fix_crash_at_run_loop branch March 2, 2020 08:32
@springmeyer
Copy link
Contributor

Thanks for this fix! I'm curious:

  • What is the root problem that this is fixing?
  • What it introduced by 9cb19b9 or was 9cb19b9 simply what uncovered a lurking problem that predated it?
  • Is there a way to write a test that would have failed before this fix (so that the problem would be caught in the gl-native tests if it regresses again, rather than only downstream)?

@springmeyer
Copy link
Contributor

@pozdnyakov @tmpsantos bump on my questions above.

@pozdnyakov
Copy link
Contributor Author

@springmeyer sorry for missing your questions!

What is the root problem that this is fixing?

The runOnce() call during run loop destruction caused the invoke() call on the same partially destructed run loop (i.e. impl->async was already deleted).

What it introduced by 9cb19b9 or was 9cb19b9 simply what uncovered a lurking problem that predated it?

the latter, it was a long-standing problem

Is there a way to write a test that would have failed before this fix (so that the problem would be caught in the gl-native tests if it regresses again, rather than only downstream)?

Yep, I'm planning to add a unit test probably next week

@springmeyer
Copy link
Contributor

Thanks @pozdnyakov - I will watch for your unit test.

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

Successfully merging this pull request may close these issues.

Segfault in mbgl::SpriteLoader::emitSpriteLoadedIfComplete
3 participants