Skip to content
New issue

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

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

Already on GitHub? # to your account

fix SAL non-determinism #1562

Merged
merged 1 commit into from
Jun 18, 2020
Merged

Conversation

weissi
Copy link
Member

@weissi weissi commented Jun 18, 2020

Motivation:

The SAL tests suffered from a rare non-determinism: When we go onto the
EventLoop to get a value and then leave it, we previously just assumed
that the EventLoop would park itself before we did any futher operations
on it.

That was mostly true because the tests are doing one thing after the
other. There was however a narrow race: If the tests enqueued new work
onto the EventLoop before the EventLoop parked itself, then the new work
would be run before the EL parks itself.

That would make the next operation fail because the operations usually
start by checking that we're parked because that's a synchronisation
point between tests and EL.

Modifications:

Actually wait until the EL's parked. That proves we found our
synchronisation point.

Result:

Motivation:

The SAL tests suffered from a rare non-determinism: When we go onto the
EventLoop to get a value and then leave it, we previously just assumed
that the EventLoop would park itself before we did any futher operations
on it.

That was mostly true because the tests are doing one thing after the
other. There was however a narrow race: If the tests enqueued new work
onto the EventLoop before the EventLoop parked itself, then the new work
would be run _before_ the EL parks itself.

That would make the _next_ operation fail because the operations usually
start by checking that we're parked because that's a synchronisation
point between tests and EL.

Modifications:

Actually wait until the EL's parked. That proves we found our
synchronisation point.

Result:

- fixes apple#1514
- fixes apple#1545
- fixes apple#1548
- fixes apple#1550
@weissi weissi requested review from glbrntt and Lukasa June 18, 2020 15:39
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jun 18, 2020
@Lukasa Lukasa added this to the 2.18.0 milestone Jun 18, 2020
Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

@weissi weissi merged commit d371bb5 into apple:master Jun 18, 2020
@weissi weissi deleted the jw-fix-1514-1545-1550-1548 branch June 18, 2020 17:04
@Lukasa Lukasa modified the milestones: 2.18.0, 2.19.0 Jun 23, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
3 participants