RFR: 8364544: Extract the checks of AtomicXxxFieldUpdater into a common place [v3]
Chen Liang
liach at openjdk.org
Tue Nov 25 04:08:21 UTC 2025
On Tue, 25 Nov 2025 03:56:10 GMT, Steve Armstrong <duke at openjdk.org> wrote:
>> The three AtomicXxxFieldUpdater classes (AtomicIntegerFieldUpdater, AtomicLongFieldUpdater, and AtomicReferenceFieldUpdater) contain duplicate field validation and access checking logic in their constructors and helper methods.
>>
>> This change extracts the common validation and utility methods into a new package-private class FieldUpdaterUtil to eliminate code duplication and improve maintainability.
>>
>> Changes:
>> - Added new FieldUpdaterUtil class with static utility methods:
>> * validateField() - validates field type, volatile, and static checks
>> * computeAccessClass() - determines correct class for access checks
>> * isSamePackage() - checks if two classes are in same package
>> * isAncestor() - checks classloader delegation chain
>>
>> - Updated AtomicIntegerFieldUpdater to use FieldUpdaterUtil
>> * Simplified constructor to use validateField() and computeAccessClass()
>> * Removed duplicate isAncestor() and isSamePackage() methods
>>
>> - Updated AtomicLongFieldUpdater to use FieldUpdaterUtil
>> * Simplified constructor to use validateField() and computeAccessClass()
>> * Removed duplicate isAncestor() and isSamePackage() methods
>>
>> - Updated AtomicReferenceFieldUpdater to use FieldUpdaterUtil
>> * Simplified constructor to use validateField() and computeAccessClass()
>> * Removed duplicate isAncestor() and isSamePackage() methods
>>
>> Existing tests in test/jdk/java/util/concurrent/tck and test/jdk/java/util/concurrent/atomic verify that the refactoring preserves the original behavior.
>
> Steve Armstrong has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>
> Refactor FieldUpdaterUtil API based on review feedback
>
> Simplified the API by combining field lookup and validation into a
> single findValidatedField() method, eliminating the need for the
> redundant FieldAndModifiers record since Field already provides
> getModifiers(). This makes the API more ergonomic and the caller
> code more concise.
Changes requested by liach (Reviewer).
src/java.base/share/classes/java/util/concurrent/atomic/FieldUpdaterUtil.java line 65:
> 63: Reflection.ensureMemberAccess(caller, tclass, null, modifiers);
> 64:
> 65: Class<?> fieldType = field.getType();
The try block must end right here. You are wrapping the IllegalArgumentException into RuntimeException incorrectly. Make sure you run
make test TEST="jdk/java/util/concurrent/tck jdk/java/util/concurrent/atomic"
tests for your changes.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28464#pullrequestreview-3503179225
PR Review Comment: https://git.openjdk.org/jdk/pull/28464#discussion_r2558475923
More information about the core-libs-dev
mailing list