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 race condition when posting CronetMetrics #2191

Merged
merged 9 commits into from
Apr 25, 2022
Merged

Fix race condition when posting CronetMetrics #2191

merged 9 commits into from
Apr 25, 2022

Conversation

carloseltuerto
Copy link
Contributor

At the end of a request, mRequestFinishedListener is used to post the CronetMetrics. Before doing so, two events must have occurred first: the "final user callback" and the "final Network callback". The Thread involved with the last of these two events is in charge of the posting the CronetMetrics - this is intrinsically racy. This PR ensures a proper behaviour.

Description: Solves CI test flakiness: unexpected java_tests_mac failure: test/java/org/chromium/net:net_tests
Risk Level: N/A (Cronet is still in development)
Testing: CI, and local multi runs
Docs Changes: N/A
Release Notes: N/A
Fixes #2136
Signed-off-by: Charles Le Borgne cleborgne@google.com

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
This allows to run the tests concurrently.

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

wow, thanks for tracking this one down!

@alyssawilk alyssawilk merged commit 8ae54b1 into main Apr 25, 2022
@alyssawilk alyssawilk deleted the cronvoy068 branch April 25, 2022 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unexpected java_tests_mac failure
2 participants