Skip to content

Integration Tests: add MainThreadMonitor to ensure main thread is not blocked#2463

Merged
NachoSoto merged 1 commit into
mainfrom
main-thread-checker
May 12, 2023
Merged

Integration Tests: add MainThreadMonitor to ensure main thread is not blocked#2463
NachoSoto merged 1 commit into
mainfrom
main-thread-checker

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

We've had a few reports of deadlocks (#2412, #2375). This might have not detected them, but it would detect future issues, as well as busy operations running on the main thread.

Example:
Screenshot 2023-05-01 at 9 50 22 AM

@NachoSoto NachoSoto added the test label May 1, 2023
@NachoSoto NachoSoto requested a review from a team May 1, 2023 16:55

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I couldn't make this a XCTFail because it wouldn't be reported if the main thread is completely blocked.

@NachoSoto NachoSoto force-pushed the main-thread-checker branch from 2e64d23 to f779252 Compare May 1, 2023 17:04

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a cool idea. Any thoughts on using this in other integration tests (not only backend integration tests)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the base class for all integration tests.

… not blocked

We've had a few reports of deadlocks (#2412, #2375). This might have not detected them, but it would detect future issues, as well as busy operations running on the main thread.
@NachoSoto NachoSoto force-pushed the main-thread-checker branch from f779252 to aee4f96 Compare May 12, 2023 19:42
@NachoSoto NachoSoto enabled auto-merge (squash) May 12, 2023 19:44
@NachoSoto NachoSoto merged commit 63b1da2 into main May 12, 2023
@NachoSoto NachoSoto deleted the main-thread-checker branch May 12, 2023 20:07
NachoSoto added a commit that referenced this pull request May 17, 2023
See #2463.
If you're actively debugging and interrupt the main thread, it'll end up asserting, which isn't very convenient.

Copied the logic from https://stackoverflow.com/a/33177600/401024. It's not the safest code, but this only runs in tests.
NachoSoto added a commit that referenced this pull request May 18, 2023
)

See #2463.
If you're actively debugging and interrupt the main thread, it'll end up
asserting, which isn't very convenient.

Copied the logic from https://stackoverflow.com/a/33177600/401024. It's
not the safest code, but this only runs in tests.
NachoSoto added a commit that referenced this pull request May 25, 2023
… not blocked (#2463)

We've had a few reports of deadlocks (#2412, #2375). This might have not
detected them, but it would detect future issues, as well as busy
operations running on the main thread.

Example:
![Screenshot 2023-05-01 at 9 50 22
AM](https://user-images.githubusercontent.com/685609/235492076-b84195ca-67e7-4762-8c56-49b1758c8534.png)
NachoSoto added a commit that referenced this pull request May 25, 2023
)

See #2463.
If you're actively debugging and interrupt the main thread, it'll end up
asserting, which isn't very convenient.

Copied the logic from https://stackoverflow.com/a/33177600/401024. It's
not the safest code, but this only runs in tests.
This was referenced May 31, 2023
NachoSoto added a commit that referenced this pull request Jul 14, 2023
This was introduced in #2463, as a way to verify the SDK didn't have any deadlocks (#2412, #2375).
However, it has caused more trouble than it's worth, because that loop keeps the main thread busy.

This solution proposed by @aboedo makes it only check every 3 seconds.
If there is a deadlock, it would still detect it. But after it's verified there is no deadlock, it won't check again until that interval has elapsed again.

This makes it so that on a test that takes 6 seconds to run, we only execute this code 2 times instead of... a lot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants