On Tue, 25 Nov 2025 03:56:10 GMT, Steve Armstrong <[email protected]> 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
