Attempt to fix deadlock issues on iOS 13

[willtr101]

No description provided.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 2 committers have signed the CLA.

❌ BusinessEngine
❌ businessengine

---

BusinessEngine seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[Adlai-Holler]

I don't think this works – leaving the group after dispatching_async defeats the purpose of the group.

[willtr101]

@Adlai-Holler Just double-checked, dispatch_group_async is just a convenient function, and it works the same way as dispatch_group_enter + dispatch_async + dispatch_group_leave ref: https://apple.github.io/swift-corelibs-libdispatch/tutorial/. So this change won't make any different comparing to the original. However, the change from dispatch_group_wait to dispatch_group_notify helped us solving the deadlock issues. Our QA is still in testing and we will investigate more to see why this only does happen on iOS 13, not on the previous iOS versions.

[Adlai-Holler]

It's not the same – dispatch_group_async is equivalent to:

group_enter()
dispatch_async(^{
  // work
  group_leave();
});

Whereas the code in this PR is different:

group_enter()
dispatch_async(^{
  // work
});
group_leave();

The group is exited before the work is finished.

[willtr101]

@Adlai-Holler can you please double-check? the group_leave is inside the dispatch_async, not outside ;-)

[Adlai-Holler]

Oh you're right. So the following two things can't both be true:

• dispatch_group_async is a shorthand for this exact code.
• Removing dispatch_group_async is necessary to fix deadlocks.

I think the first one is true. So perhaps it's switching from wait to notify that is fixing your deadlocks (seems likely.)

[willtr101]

Yes first one is true and switching from wait to notify fixed our deadlocks. We are confident to release our app with that change as we noticed no crashes/app being frozen issues since that fix. We have to comment an assert due to the notify seems working differently with the wait.

[Adlai-Holler]

Hey @businessengine could you elaborate on the deadlock you're seeing? The current code as-written breaks the semantics of the editing transaction queue.