[
https://issues.apache.org/jira/browse/TIKA-2667?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16518490#comment-16518490
]
Uwe Schindler edited comment on TIKA-2667 at 6/20/18 7:04 PM:
--------------------------------------------------------------
It's OK because it wont fail, but I dont understand the need to catch Throwable
and the reason to use AtomicReference. The doPrivileged part cannot throw any
exception it will always succeed, all exceptions are handled internally! Do
privileged is not risky as it does not do something like "sudo" (the name of
method is misleading). It just executes the stuff inside the lambda with the
privileges of the current code base (and that's documented to be always
possible, because we call the method ourselves). Without the doPrivileged it
would call the stuff with caller's privileges. The doPrivileged call is there
to allow user of the JAR to configure the JVM that only our JAR file can do the
privileged action. This improves security, because you don't need to give
everyone the permission to call setAccesible() and access Unsafe. It's only
important to NOT give the MethodHandle to untrusted code, so it must be
"private final".
So just copypaste the whole code from Lucene's MMAP directory - the static
initializer (maybe do code a bit different with error reporting, Lucene uses no
logging):
https://github.com/apache/lucene-solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L312-L336
The return value of the method reference to the private unmapper method is
Object to allow to pass a String or a MethodHandle through the return value of
the privileged block: The code you added using the AtomicReference is not
needed - exactly because the privileged code returns either a method handle OR
it returns an error message (that's the trick).
The resourceDescription is used to make the exception more meaningful (in
Lucene we use filename, so user get's an error about what file handle caused
the issue).
This trycatch in the code is obsolete:
https://github.com/tballison/jmatio/blob/master/src/main/java/com/jmatio/io/MatFileReader.java#L376-L395
The BufferCleaner interface just throws an IOException if unmapping goes wrong
- with a meaningful error message. So I'd remove the try-catch block, it's
legacy.
Maybe I should create a Pull Request? Unfortunately I have no time and no
checkout of the matfile reader ready....
was (Author: thetaphi):
It's OK because it wont fail, but I dont understand the need to catch Throwable
and the reason to use AtomicReference. The doPrivileged part cannot throw any
exception it will always succeed, all exceptions are handled internally! Do
privileged is not risky as it does not do something like "sudo" (the name of
method is misleading). It just executes the stuff inside the lambda with the
privileges of the current code base (and that's documented to be always
possible, because we call the method ourselves). Without the doPrivileged it
would call the stuff with caller's privileges. The doPrivileged call is there
to allow user of the JAR to configure the JVM that only our JAR file can do the
privileged action. This improves security, because you don't need to give
everyone the permission to call setAccesible() and access Unsafe.
So just copypaste the whole code from Lucene's MMAP directory - the static
initializer (maybe do code a bit different with error reporting, Lucene uses no
logging):
https://github.com/apache/lucene-solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L312-L336
The return value of the method reference to the private unmapper method is
Object to allow to pass a String or a MethodHandle through the return value of
the privileged block: The code you added using the AtomicReference is not
needed - exactly because the privileged code returns either a method handle OR
it returns an error message (that's the trick).
The resourceDescription is used to make the exception more meaningful (in
Lucene we use filename, so user get's an error about what file handle caused
the issue).
This trycatch in the code is obsolete:
https://github.com/tballison/jmatio/blob/master/src/main/java/com/jmatio/io/MatFileReader.java#L376-L395
The BufferCleaner interface just throws an IOException if unmapping goes wrong
- with a meaningful error message. So I'd remove the try-catch block, it's
legacy.
Maybe I should create a Pull Request? Unfortunately I have no time and no
checkout of the matfile reader ready....
> Upgrade jmatio to 1.4
> ----------------------
>
> Key: TIKA-2667
> URL: https://issues.apache.org/jira/browse/TIKA-2667
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Trivial
>
> jmatio 1.3 includes an upgrade to clean MappedByteBuffers in Java 8->11-ea,
> thanks to a copy/paste from Lucene.
> jmatio 1.4 will include one that actually works. Thank you, [~thetaphi]!
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
