RFR: 8350549: MethodHandleProxies.WRAPPER_TYPES is not thread-safe [v2]

[Jaikiran Pai]

On Wed, 30 Apr 2025 23:02:24 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Use a thread-safe ReferencedKeySet instead of a WeakHashMap key set.
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Add a test case, fails isWrapperInstance on mainline
>  - Merge branch 'master' of https://github.com/openjdk/jdk into fix/mhp-wrapper-check-async
>  - 8350549: MethodHandleProxies.WRAPPER_TYPES is not thread-safe

Thank you Chen for adding the test. The test looks OK to me.

Please add `8350549` to the `@bug` in the test definition.

I think a brief comment on the test method would make it easier to understand what it does. Would you consider adding a comment, something like:


/**
 * The test verifies that MethodHandleProxies.asInterfaceInstance() and
 * MethodHandleProxies.isWrapperInstance() work as expected when invoked concurrently.
 */



Finally, given the nature of this new test, I think it would be good to have it run in our CI with a test repeat of around 50 to be sure that it doesn't fail intermittently.

test/jdk/java/lang/invoke/MethodHandleProxies/BasicTest.java line 275:

> 273:                 .limit(100)
> 274:                 .forEach(inst -> assertTrue(MethodHandleProxies.isWrapperInstance(inst),
> 275:                         () -> Objects.toIdentityString(inst)));

Nit - the failure message (if at all it fails for any reason) could perhaps be:


Objects.toIdentityString(inst) + " was expected to be a wrapper instance, but isn't"

-------------

PR Review: https://git.openjdk.org/jdk/pull/23757#pullrequestreview-2810048465
PR Review Comment: https://git.openjdk.org/jdk/pull/23757#discussion_r2070418275