GH-1007: fix: does not break class loading if direct buffer allocator is not available#1008
GH-1007: fix: does not break class loading if direct buffer allocator is not available#1008torito wants to merge 5 commits intoapache:mainfrom
Conversation
torito
requested review from
jbonofre,
laurentgo,
lidavidm and
wgtmac
as code owners
This comment has been minimized.
This comment has been minimized.
|
I'm not able to set the |
|
@lidavidm can you review this please |
|
@torito I gonna take a look. By the way, I'm fixing the CI, so the build for this PR will probably fail. |
|
@torito can you please rebase this PR ? The CI should be ok now. |
torito
force-pushed
the
catch-jpms-failure
branch
from
115e465 to
b002db5
Compare
| addressField.setAccessible(true); | ||
| maybeOffset = UNSAFE.objectFieldOffset(addressField); | ||
| } catch (InaccessibleObjectException e) { | ||
| maybeOffset = -1; |
There was a problem hiding this comment.
I understand the rationale here: we try first with the address field offset, and if it fails (InaccessibleObjectException) we fallback to offset -1 and try again. It can fail again (InaccessibleObjectException), and then we throw the exception.
I don't see any side effect, but it's potential a kind of breaking change on the allocation.
It looks good to me, but I would love to have four eyes validation by @lidavidm
There was a problem hiding this comment.
I guess my question is, what is the value of deferring the exception until later? I'm not sure you can do much with arrow-java if this isn't available.
3 participants
What's Changed
The Direct Buffer is not always needed to use Arrow memory, however, we cannot load MemoryUtil class if we don't set:
Which is not always needed/possible.
This fix proposes to catch the
InaccessibleObjectExceptionto not avoiding the load of the class.The directBuffer is, in any case not available and a
UnsupportedOperationExceptionwill be throw as it is in the existing codeCloses #1007 .