Fix(15831): Resolve race condition in ReferenceManager.doMaybeRefresh#15985
Fix(15831): Resolve race condition in ReferenceManager.doMaybeRefresh#15985vijaykriishna wants to merge 1 commit into
Conversation
vijaykriishna
force-pushed
the
fix/15831
branch
from
240d639 to
0f53b57
Compare
vigyasharma
left a comment
vigyasharma
left a comment
There was a problem hiding this comment.
Changes look good, thanks @vijaykriishna !
|
|
||
| * GITHUB#15939: Fix thread-safety issues with NFARunAutomaton. (Dimitris Rempapis) | ||
|
|
||
| * GITHUB#15831: Fixed a race condition in ReferenceManager.maybeRefresh() where refreshes could be missed if a new generation was flushed just before the refresh lock was released. (Vijay) |
There was a problem hiding this comment.
This can go out with 10.5 I think. Let's move this entry to under the 10.5.0 section?
There was a problem hiding this comment.
Is it only the entry in changes.txt, or the code change as well?
There was a problem hiding this comment.
I suspect @vigyasharma meant the code as well -- when we merge, we will cherry-pick to backport, or if that's conflicty, open a separate backport PR. Thanks @vijaykriishna, this is a good catch -- I'm curious how you caught this (the opened issue)?
There was a problem hiding this comment.
Hi @vijaykriishna : the changes entry for each contribution lives in the release version it's targeted for. It is helpful then, in the release process, to go over the CHANGES file and see what is getting released. Typically, you'll find that the changes file has two upcoming release sections, one for the next major version (11.0.0 right now) and one for the next minor version (10.5.0).
This change is backward compatible and doesn't break any APIs and can go in the next minor version release i.e. 10.5.0. So your changes entry will belong under the 10.5.0 header.
There was a problem hiding this comment.
Updated CHANGES.txt to make sure the fix is targeted for 10.5.0. Please review. If the code change needs to be put into 10.5.0, branch_10x is the target branch I believe.
3 participants
Description
The key behavioral change is that with the loop in doMaybeRefresh(), a single call to maybeRefreshBlocking() will now refresh through multiple generations until the reference is fully up to date. This ensures that pending refreshes are not lost due to timing races.
Resolves #15831