Move polymorphic VarHandle calls to separate class to avoid Android v…#15661
Conversation
…erification error Motivation: Android 8–12 ART/Dalvik rejects classes with direct signature-polymorphic VarHandle call sites. After upgrading Netty to 4.2.4+, UnpooledDirectByteBuf triggers a java.lang.VerifyError when its methods reference ByteBuffer VarHandle view operations, even if VarHandles are disabled via -Dio.netty.varHandle.enabled=false. As suggested by yawkat in #15654, the fix is to move VarHandle usage into a separate class and only load it when VarHandles are actually available. Modification: - Introduce VarHandleByteBufferAccess to centralize all ByteBuffer VarHandle get/set operations. - Update UnpooledDirectByteBuf to: - Guard VarHandle paths with PlatformDependent.hasVarHandle(). - Delegate to VarHandleByteBufferAccess for get/set of short/int/long (BE/LE). - Fall back to ByteBuffer methods (and byte-order swaps) when VarHandles are unavailable. - Remove direct VarHandle import to avoid embedding signature-polymorphic call sites in this class. - No behavioral changes on platforms with working VarHandles; Android 8–12 avoids verifier failures by not loading the helper class. Touched methods in UnpooledDirectByteBuf: _getShort, _getShortLE, _getInt, _getIntLE, _getLong, _getLongLE, _setShort, _setShortLE, _setInt, _setIntLE, _setLong, _setLongLE. Result: Fixes #15654.
franz1981
left a comment
There was a problem hiding this comment.
Looks good to me, it shouldn't change behaviour for hotspot, if not that we have another method to compile but very minimal (likely in the small inline level)
|
@yawkat I think we still need to disable VarHandles on android by default imho |
|
I'm not sure, I think the check that's already in place should be enough. A runtime that doesn't support polymorphic signatures for VarHandle is also unlikely to support VarHandle.storeStoreFence. |
|
@yawkat maybe... maybe better be save then sorry ? But lets wait for the reporter to test it |
|
I think this is good to go... Happy to merge ? |
|
If you want to we can merge it speculatively, and have @ajith211 test with the snapshot. This PR at least shouldn't hurt anyone. But I don't know if it really works. |
|
@yawkat yeah I think this would be the best... let me merge it |
If you can share a testable artifact, we can consume and verify the same. |
|
Going forward, do we need to disable this varHandle flag on Android builds ? |
Just try 4.2.7-SNAPSHOT |
no |
Applied <netty.version>4.2.7-SNAPSHOT</netty.version> |
We use maven central for snapshots: |
|
@ajith211 please keep us posted |
|
Not able to consume this artifact yet. [WARNING] Failed to download maven-metadata-central-portal-snapshots1.xml [https://central.sonatype.com/repositories/snapshots/] |
|
Not |
|
Not its '4.2.7.Final-SNAPSHOT'. As you can see it was deployed: https://github.com/netty/netty/actions/runs/17756760347/job/50461461860 |
|
Could not find artifact io.netty:netty-codec-socks:jar:4.2.7-SNAPSHOT in central-portal-snapshots (https://central.sonatype.com/repository/maven-snapshots/) |
|
alternative just install it |
I am bit confused about the snapshot artifact by seeing below log |
|
I could able to consume 4.2.7.Final-SNAPSHOT but the problem is still present with this version. |
|
Can you paste the stacktrace / error? |
|
Rejecting re-init on previously-failed class java.lang.Class<io.netty.buffer.HeapByteBufUtil>: java.lang.VerifyError: Verifier rejected class io.netty.buffer.HeapByteBufUtil: int io.netty.buffer.HeapByteBufUtil.getInt(byte[], int) failed to verify: int io.netty.buffer.HeapByteBufUtil.getInt(byte[], int): [0xF] int io.netty.buffer.HeapByteBufUtil.getInt(byte[], int): [0xF] register v5 has type Precise Reference: byte[] but expected Reference: java.lang.Object[] |
|
Oh, that error mentions a different class, HeapByteBufUtil. I think this is good news – the fix likely worked, but I missed some call sites. I will try to throw AI at that problem too :) |
Motivation: Follow-up to #15661. Move signature-polymorphic VarHandle calls out of HeapByteBufUtil to avoid class verification issues on older Android runtimes when VarHandle is disabled. Modifications: - Add byte[] getters/setters to VarHandleByteBufferAccess that wrap PlatformDependent.*ArrayView() VarHandles. - Use these methods from HeapByteBufUtil when VarHandle is enabled, preserving existing bitwise fallbacks otherwise. - Remove direct signature-polymorphic calls from HeapByteBufUtil. Result: No direct references to signature-polymorphic VarHandle methods remain in HeapByteBufUtil. Behavior is unchanged when VarHandle is disabled.
…ss (#15670) Motivation: Follow-up to #15661. Move signature-polymorphic VarHandle calls out of HeapByteBufUtil to avoid class verification issues on older Android runtimes when VarHandle is disabled. Modifications: - Add byte[] getters/setters to VarHandleByteBufferAccess that wrap PlatformDependent.*ArrayView() VarHandles. - Use these methods from HeapByteBufUtil when VarHandle is enabled, preserving existing bitwise fallbacks otherwise. - Remove direct signature-polymorphic calls from HeapByteBufUtil. Result: No direct references to signature-polymorphic VarHandle methods remain in HeapByteBufUtil. Behavior is unchanged when VarHandle is disabled.
…erification error
Motivation:
Android 8–12 ART/Dalvik rejects classes with direct signature-polymorphic VarHandle call sites. After upgrading Netty to 4.2.4+, UnpooledDirectByteBuf triggers a java.lang.VerifyError when its methods reference ByteBuffer VarHandle view operations, even if VarHandles are disabled via -Dio.netty.varHandle.enabled=false. As suggested by yawkat in #15654, the fix is to move VarHandle usage into a separate class and only load it when VarHandles are actually available.
Modification:
Touched methods in UnpooledDirectByteBuf: _getShort, _getShortLE, _getInt, _getIntLE, _getLong, _getLongLE, _setShort, _setShortLE, _setInt, _setIntLE, _setLong, _setLongLE.
Result:
Fixes #15654.
I threw AI at the problem and told it to fix the issue according to my comment on the issue thread. @ajith211, please try this fix. You'll have to build netty locally to test it though.