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.
There was a problem hiding this comment.
Hi, we can still use arrow and some internals, we are using it in production but we are maintaining a fork to be able to load the classes. Otherwise, the class simply does not load, and by consequence our classes are not loaded neither. My main problem is we cannot control the JVM and set the --add-opens, as our application is a plugin which is installed in clusters, where we cannot control jvm initialization, sometimes in the cloud, which is even more difficult to set --add-opens
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 .