[tryout fix] concurrency bug in NamespacedHierarchicalStore#computeIfAbsent(Object, Object, Function) #5171 #5209
#5223
+918
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ All tests passed ✅Test Summary
🏷️ Commit: 0f9c064 Test FailuresNamespacedHierarchicalStoreTests > ConcurrencyIssue5171 > computeIfAbsentWithMultipleExceptionsAndConcurrentModifications() (:platform-tests:test in OpenJ9 21)NamespacedHierarchicalStoreTests > ConcurrencyIssue5171 > computeIfAbsentWithMultipleExceptionsAndConcurrentModifications() (:platform-tests:test in OpenJDK 26 (ea))NamespacedHierarchicalStoreTests > ConcurrencyIssue5171 > computeIfAbsentWithMultipleExceptionsAndConcurrentModifications() (:platform-tests:test in OpenJDK 26 (ea))Learn more about TestLens at testlens.app. |
Pankraz76
commented
Dec 13, 2025
| public <K, V> Object computeIfAbsent(N namespace, K key, Function<? super K, ? extends V> defaultCreator) { | ||
| Preconditions.notNull(defaultCreator, "defaultCreator must not be null"); | ||
| CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key); | ||
| StoredValue storedValue = getStoredValue(compositeKey); |
Contributor
Author
There was a problem hiding this comment.
I see that it is harder to review. On the other hand, it is also difficult (for me) to access this code without. This can be helpful when working on a close, low-level perspective.
I do not consider code at all, because everything ultimately becomes some kind of random implementation detail.
If I need to consider code seriously, I must structure it properly and understand how it actually flows, while adhering to best practices and SOLID design principles that reflect what the code is truly doing and how it whispers how it wants to be structured. I know not everyone has the sense to recognize this level of intent.
I cannot process an endless number of requirements and concerns. I can only operate like a computer: one thing at a time, within a very limited scope—effectively binary. While 0 and 1 represent two states, they never truly add up to 2. Because of this, I have to work at a one-to-one level; anything beyond that exceeds my strict mental limitations. Please excuse my shortcomings in this regard.
Contributor
Author
|
kindly asking if this is any good and or any better? Of course will undo the unrelated var stuff if you demand it. |
Pankraz76
commented
Dec 13, 2025
...ngine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java
Show resolved
Hide resolved
Pankraz76
force-pushed
the
fix-storeNewValue
branch
from
02e511d to
d87f48c
Compare
Contributor
Author
|
kindly asking how to link local junit version with the assertj project? Also can not find the tag to activate failing regression test. Thanks. |
…fAbsent(Object, Object, Function)` junit-team#5171 junit-team#5209 Signed-off-by: Vincent Potucek <[email protected]>
Pankraz76
force-pushed
the
fix-storeNewValue
branch
from
d87f48c to
0f9c064
Compare
Pankraz76
commented
Dec 13, 2025
| @Nullable | ||
| private volatile Object value = NO_VALUE_SET; | ||
|
|
||
| private MemoizingSupplier(Supplier<@Nullable Object> delegate) { |
Contributor
Author
There was a problem hiding this comment.
seems like a typo missing r
Contributor
There was a problem hiding this comment.
They're similar words but mean different things.
Contributor
Author
|
strange like this one stable flaky when running with TC: org.junit.platform.engine.support.store.NamespacedHierarchicalStoreTests.ConcurrencyIssue5171#computeIfAbsentWithMultipleExceptionsAndConcurrentModifications |
mpkorstanje
reviewed
Dec 14, 2025
Contributor
There was a problem hiding this comment.
Thanks for trying to fix this! But this still appears to be based on #5209 and that pull request was based on an incomplete understanding of the problem.
I've given what I think is a complete description of the actual problem in #5171. And while I think I know what the problem is I haven't quite had the time available to work out what the solution should be. I also don't have the time available to coach you towards an understanding of the problem to such a degree that you can develop a solution.
As such, and in addition to the reasons given in #5213 (review), I don't think it a good use of your time to submit pull requests for this issue.
| @Nullable | ||
| private volatile Object value = NO_VALUE_SET; | ||
|
|
||
| private MemoizingSupplier(Supplier<@Nullable Object> delegate) { |
Contributor
There was a problem hiding this comment.
They're similar words but mean different things.
Contributor
Author
Yes, your assumption is correct. Unfortunately, I did not provide enough context earlier, and I should have explained this better — please excuse my shortcomings. I understood your previous comments: the tests and overall structure are fine, but they lack detailed focus on the actual fix and include changes that address unrelated flaws as well. My intention was to remove the unnecessary parts and keep only what seemed worthwhile to discuss or investigate further. I would also expect tests that are logically related to be grouped together in practice, rather than scattered throughout a test file that is already over 1,000 lines long. Grouping them using nested test classes makes it much easier to access and reason about the specific issues being addressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[tryout fix] concurrency bug in
NamespacedHierarchicalStore#computeIfAbsent(Object, Object, Function)#5171 #5209I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotationscoverage:
before:
after: