Java 9+ modules / cleaner

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Java 9+ modules / cleaner

kiwiwings
Hi *,

I'm facing the same problem as described in https://issues.apache.org/jira/browse/LUCENE-6989

Is it ok for you, if I get our build more or less to run with the module path (instead of the classpath) when running in a JDK 9+ and later try to fix the above Cleaner problem?

I simply would like to focus on one issue now and as we a have multi-release jars after my commit, a JDK dependent solution shouldn't be a problem anymore.

Andi



signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

Dominik Stadler
Hi,

Sorry for replying late here, not sure if you already did these changes,
but the code in CleanerUtil tries to handle this gracefully with not
cleaning on JDKs which do not support this.

There should be no compile-time dependency on any unsafe object, during
runtime we try to get a cleaner and simply not unmap buffers cleanly if not
possible for some reason.

Which new problem do you see with this approach when using the JDK module
system?

Thanks... Dominik.

On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker <[hidden email]>
wrote:

> Hi *,
>
> I'm facing the same problem as described in
> https://issues.apache.org/jira/browse/LUCENE-6989
>
> Is it ok for you, if I get our build more or less to run with the module
> path (instead of the classpath) when running in a JDK 9+ and later try to
> fix the above Cleaner problem?
>
> I simply would like to focus on one issue now and as we a have
> multi-release jars after my commit, a JDK dependent solution shouldn't be a
> problem anymore.
>
> Andi
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

kiwiwings
Hi Dominik,

the goal is to have no --add-opens or similar jvm arguments. In this case we get the following exception:

   [junit] java.lang.reflect.InaccessibleObjectException: Unable to make public jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible: module java.base does not "opens java.nio" to module org.apache.poi.poi
    [junit]     at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:349)
    [junit]     at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:289)
    [junit]     at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
    [junit]     at java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(CleanerUtil.java:116)
    [junit]     at java.base/java.security.AccessController.doPrivileged(AccessController.java:312)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.java:77)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileBackedDataSource.java:189)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$close$0(FileBackedDataSource.java:162)
    [junit]     at java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBackedDataSource.java:162)
    [junit]     at org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFSFileSystem.java:764)
    [junit]     at org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(TestWrite.java:539)

If we use the Runnable aproach mentioned in the lucene bug, this should work again.

In the meantime I had to move a few sources from ooxml to main (crypto.agile) and we need to find a way, e.g. using multi release classes, for our WorkbookFactory, as with JIgsaw the poi main module can't reflect into ooxml anymore. This was also a reason for moving the agile crypto into poi main and removing the use XmlBeans schemas there.

Best wishes,
Andi


On 07.07.20 18:00, Dominik Stadler wrote:

> Hi,
>
> Sorry for replying late here, not sure if you already did these changes,
> but the code in CleanerUtil tries to handle this gracefully with not
> cleaning on JDKs which do not support this.
>
> There should be no compile-time dependency on any unsafe object, during
> runtime we try to get a cleaner and simply not unmap buffers cleanly if not
> possible for some reason.
>
> Which new problem do you see with this approach when using the JDK module
> system?
>
> Thanks... Dominik.
>
> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker <[hidden email]>
> wrote:
>
>> Hi *,
>>
>> I'm facing the same problem as described in
>> https://issues.apache.org/jira/browse/LUCENE-6989
>>
>> Is it ok for you, if I get our build more or less to run with the module
>> path (instead of the classpath) when running in a JDK 9+ and later try to
>> fix the above Cleaner problem?
>>
>> I simply would like to focus on one issue now and as we a have
>> multi-release jars after my commit, a JDK dependent solution shouldn't be a
>> problem anymore.
>>
>> Andi
>>
>>
>>



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

kiwiwings
Hi Uwe,

I currently try to understand, how to call the Cleaner in Java 14 (or 9+) without adding the --add-opens JVM options.
As you've worked on this in LUCENE-6989, you might have a few hints for me.

I've checked the Lucene implementation, but that code is similar to POIs current implementation. [1]
As I don't see the Runable interface, I might look at the wrong branch.

Any ideas?

Best wishes,
Andi


[1]
https://github.com/apache/lucene-solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L338
vs.
https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/CleanerUtil.java?view=markup#l91

On 09.07.20 00:34, Andreas Beeker wrote:

> Hi Dominik,
>
> the goal is to have no --add-opens or similar jvm arguments. In this case we get the following exception:
>
>    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make public jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible: module java.base does not "opens java.nio" to module org.apache.poi.poi
>     [junit]     at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:349)
>     [junit]     at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:289)
>     [junit]     at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
>     [junit]     at java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(CleanerUtil.java:116)
>     [junit]     at java.base/java.security.AccessController.doPrivileged(AccessController.java:312)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.java:77)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileBackedDataSource.java:189)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$close$0(FileBackedDataSource.java:162)
>     [junit]     at java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBackedDataSource.java:162)
>     [junit]     at org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFSFileSystem.java:764)
>     [junit]     at org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(TestWrite.java:539)
>
> If we use the Runnable aproach mentioned in the lucene bug, this should work again.
>
> In the meantime I had to move a few sources from ooxml to main (crypto.agile) and we need to find a way, e.g. using multi release classes, for our WorkbookFactory, as with JIgsaw the poi main module can't reflect into ooxml anymore. This was also a reason for moving the agile crypto into poi main and removing the use XmlBeans schemas there.
>
> Best wishes,
> Andi
>
>
> On 07.07.20 18:00, Dominik Stadler wrote:
>> Hi,
>>
>> Sorry for replying late here, not sure if you already did these changes,
>> but the code in CleanerUtil tries to handle this gracefully with not
>> cleaning on JDKs which do not support this.
>>
>> There should be no compile-time dependency on any unsafe object, during
>> runtime we try to get a cleaner and simply not unmap buffers cleanly if not
>> possible for some reason.
>>
>> Which new problem do you see with this approach when using the JDK module
>> system?
>>
>> Thanks... Dominik.
>>
>> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker <[hidden email]>
>> wrote:
>>
>>> Hi *,
>>>
>>> I'm facing the same problem as described in
>>> https://issues.apache.org/jira/browse/LUCENE-6989
>>>
>>> Is it ok for you, if I get our build more or less to run with the module
>>> path (instead of the classpath) when running in a JDK 9+ and later try to
>>> fix the above Cleaner problem?
>>>
>>> I simply would like to focus on one issue now and as we a have
>>> multi-release jars after my commit, a JDK dependent solution shouldn't be a
>>> problem anymore.
>>>
>>> Andi
>>>
>>>
>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler-3
Hi,

> I currently try to understand, how to call the Cleaner in Java 14 (or 9+) without
> adding the --add-opens JVM options.

Yeah, your code won't work correctly with Java 9 at all. You may fix it with some opens, but still types of internal calsses changed, so its just risky (everything is subject to change).

> As you've worked on this in LUCENE-6989, you might have a few hints for me.

I think you can more or less copy the code from Lucene (use the branch_8x version, as Master requires Java 11, so has no Java 8 code anymore, see here: https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use Reflection at all, it works with Method Handles. The important thing is: Method handles are linked earyl, once you have built the final method handle to invoke the cleaner, you can call it without requesting any additional right (security manager). In addition you know beforehand if it works at all (it cannot throw extra reflective exceptions).

The main change in Java 9 is (and this is what's officially "supported" by OpenJDK developer): They added a new method to sun.misc.Unsafe (the legacy one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base and is open to public (if you know how to get the singleton) for the time being. To get the singleton, you need reflection and the code must allow to do setAccessible on the getter, but once you have it, it's useable.

To unmap a Bytebuffer, Lucene creates a MethodHandle with some signature like "unmap(ByteBuffer b)" complete pre-configued on startup of class depending on Java version:
- In Java 9 and above it uses reflection to get the Unsafe instance (this requires security manager to allow it). Then it looks up the method "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the final methodhandle is casted to me "void unmap(ByteBuffer)"
- In Java 8 and before it uses more or less the old approach by checking the private method to get the Cleaner instance. Finally it calls cleaner.clean(). This can also be composed to a MethodHandle with exactly same signature (using the famous MethodHandle transformation and bindings, introducing some null checks). The result is also a methodhandle with signature "void unmap(ByteBuffer)".

Once all this is done, the methodhandle with platform independent signature can be called without any exception handling from any code, so be sure to keep it safe in private final fields fully internal to your implementaion (otherwise it's a security issue).

> I've checked the Lucene implementation, but that code is similar to POIs
> current implementation. [1]
> As I don't see the Runable interface, I might look at the wrong branch.

That won't work. It's the same approach like the old one, just with other class types. You cant work around the internal Cleaner interface. This is why sun.misc.Unsafe#invokeCleaner() was added.

> Any ideas?

See above. I'd copy the code from Lucene: It's early binding and failsafe, once you got the MethodHandle. The approach should work with Java 7+. In Java 7 there is one additional helper method needed for the methodHandle regarding the null check!

Java 8 is here: https://tinyurl.com/y47euqfg
Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k (this adds a "compatibility method" to replace missing "Objects#nonNull(Object)", but it's identical otherwise; we added this to make Lucene 5.5 still compatible with Java 9, long after support ended, because customers need this).

Uwe

> Best wishes,
> Andi
>
>
> [1]
> https://github.com/apache/lucene-
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> ry.java#L338
> vs.
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> anerUtil.java?view=markup#l91
>
> On 09.07.20 00:34, Andreas Beeker wrote:
> > Hi Dominik,
> >
> > the goal is to have no --add-opens or similar jvm arguments. In this case we
> get the following exception:
> >
> >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make public
> jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible: module
> java.base does not "opens java.nio" to module org.apache.poi.poi
> >     [junit]     at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> bject.java:349)
> >     [junit]     at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> bject.java:289)
> >     [junit]     at
> java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> >     [junit]     at
> java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> rUtil.java:116)
> >     [junit]     at
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> )
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> a:77)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> ackedDataSource.java:189)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> e$0(FileBackedDataSource.java:162)
> >     [junit]     at
> java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> kedDataSource.java:162)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> FileSystem.java:764)
> >     [junit]     at
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> Write.java:539)
> >
> > If we use the Runnable aproach mentioned in the lucene bug, this should
> work again.
> >
> > In the meantime I had to move a few sources from ooxml to main
> (crypto.agile) and we need to find a way, e.g. using multi release classes, for
> our WorkbookFactory, as with JIgsaw the poi main module can't reflect into
> ooxml anymore. This was also a reason for moving the agile crypto into poi
> main and removing the use XmlBeans schemas there.
> >
> > Best wishes,
> > Andi
> >
> >
> > On 07.07.20 18:00, Dominik Stadler wrote:
> >> Hi,
> >>
> >> Sorry for replying late here, not sure if you already did these changes,
> >> but the code in CleanerUtil tries to handle this gracefully with not
> >> cleaning on JDKs which do not support this.
> >>
> >> There should be no compile-time dependency on any unsafe object, during
> >> runtime we try to get a cleaner and simply not unmap buffers cleanly if not
> >> possible for some reason.
> >>
> >> Which new problem do you see with this approach when using the JDK
> module
> >> system?
> >>
> >> Thanks... Dominik.
> >>
> >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> <[hidden email]>
> >> wrote:
> >>
> >>> Hi *,
> >>>
> >>> I'm facing the same problem as described in
> >>> https://issues.apache.org/jira/browse/LUCENE-6989
> >>>
> >>> Is it ok for you, if I get our build more or less to run with the module
> >>> path (instead of the classpath) when running in a JDK 9+ and later try to
> >>> fix the above Cleaner problem?
> >>>
> >>> I simply would like to focus on one issue now and as we a have
> >>> multi-release jars after my commit, a JDK dependent solution shouldn't be
> a
> >>> problem anymore.
> >>>
> >>> Andi
> >>>
> >>>
> >>>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler
Hi,

sorry I did not read all, so you already use my code:

The exception you see is coming from the fact that the first part for Java 9 does not work at all. It looks like the code uses Java 8 variant (see line number in exception) instead of the Java 9 variant.

Can you make a breakpoint here: https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/CleanerUtil.java?view=markup#l99

And step through the code? It might fall through to the catch block and java 8 code.

Uwe

-----
Uwe Schindler
Achterdiek 19, D-28357 Bremen
https://www.thetaphi.de
eMail: [hidden email]

> -----Original Message-----
> From: Uwe Schindler <[hidden email]>
> Sent: Friday, August 7, 2020 5:06 PM
> To: 'Andreas Beeker' <[hidden email]>; [hidden email]
> Cc: [hidden email]
> Subject: RE: Java 9+ modules / cleaner
>
> Hi,
>
> > I currently try to understand, how to call the Cleaner in Java 14 (or 9+)
> without
> > adding the --add-opens JVM options.
>
> Yeah, your code won't work correctly with Java 9 at all. You may fix it with
> some opens, but still types of internal calsses changed, so its just risky
> (everything is subject to change).
>
> > As you've worked on this in LUCENE-6989, you might have a few hints for me.
>
> I think you can more or less copy the code from Lucene (use the branch_8x
> version, as Master requires Java 11, so has no Java 8 code anymore, see here:
> https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use Reflection
> at all, it works with Method Handles. The important thing is: Method handles
> are linked earyl, once you have built the final method handle to invoke the
> cleaner, you can call it without requesting any additional right (security
> manager). In addition you know beforehand if it works at all (it cannot throw
> extra reflective exceptions).
>
> The main change in Java 9 is (and this is what's officially "supported" by
> OpenJDK developer): They added a new method to sun.misc.Unsafe (the legacy
> one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base and is
> open to public (if you know how to get the singleton) for the time being. To get
> the singleton, you need reflection and the code must allow to do setAccessible
> on the getter, but once you have it, it's useable.
>
> To unmap a Bytebuffer, Lucene creates a MethodHandle with some signature
> like "unmap(ByteBuffer b)" complete pre-configued on startup of class
> depending on Java version:
> - In Java 9 and above it uses reflection to get the Unsafe instance (this requires
> security manager to allow it). Then it looks up the method
> "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the final
> methodhandle is casted to me "void unmap(ByteBuffer)"
> - In Java 8 and before it uses more or less the old approach by checking the
> private method to get the Cleaner instance. Finally it calls cleaner.clean(). This
> can also be composed to a MethodHandle with exactly same signature (using
> the famous MethodHandle transformation and bindings, introducing some null
> checks). The result is also a methodhandle with signature "void
> unmap(ByteBuffer)".
>
> Once all this is done, the methodhandle with platform independent signature
> can be called without any exception handling from any code, so be sure to keep
> it safe in private final fields fully internal to your implementaion (otherwise it's
> a security issue).
>
> > I've checked the Lucene implementation, but that code is similar to POIs
> > current implementation. [1]
> > As I don't see the Runable interface, I might look at the wrong branch.
>
> That won't work. It's the same approach like the old one, just with other class
> types. You cant work around the internal Cleaner interface. This is why
> sun.misc.Unsafe#invokeCleaner() was added.
>
> > Any ideas?
>
> See above. I'd copy the code from Lucene: It's early binding and failsafe, once
> you got the MethodHandle. The approach should work with Java 7+. In Java 7
> there is one additional helper method needed for the methodHandle regarding
> the null check!
>
> Java 8 is here: https://tinyurl.com/y47euqfg
> Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k (this
> adds a "compatibility method" to replace missing "Objects#nonNull(Object)",
> but it's identical otherwise; we added this to make Lucene 5.5 still compatible
> with Java 9, long after support ended, because customers need this).
>
> Uwe
>
> > Best wishes,
> > Andi
> >
> >
> > [1]
> > https://github.com/apache/lucene-
> >
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> > ry.java#L338
> > vs.
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > anerUtil.java?view=markup#l91
> >
> > On 09.07.20 00:34, Andreas Beeker wrote:
> > > Hi Dominik,
> > >
> > > the goal is to have no --add-opens or similar jvm arguments. In this case we
> > get the following exception:
> > >
> > >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make
> public
> > jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible: module
> > java.base does not "opens java.nio" to module org.apache.poi.poi
> > >     [junit]     at
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > bject.java:349)
> > >     [junit]     at
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > bject.java:289)
> > >     [junit]     at
> > java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> > >     [junit]     at
> > java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> > rUtil.java:116)
> > >     [junit]     at
> >
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> > )
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> > a:77)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> > ackedDataSource.java:189)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> > e$0(FileBackedDataSource.java:162)
> > >     [junit]     at
> > java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> > kedDataSource.java:162)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> > FileSystem.java:764)
> > >     [junit]     at
> >
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> > Write.java:539)
> > >
> > > If we use the Runnable aproach mentioned in the lucene bug, this should
> > work again.
> > >
> > > In the meantime I had to move a few sources from ooxml to main
> > (crypto.agile) and we need to find a way, e.g. using multi release classes, for
> > our WorkbookFactory, as with JIgsaw the poi main module can't reflect into
> > ooxml anymore. This was also a reason for moving the agile crypto into poi
> > main and removing the use XmlBeans schemas there.
> > >
> > > Best wishes,
> > > Andi
> > >
> > >
> > > On 07.07.20 18:00, Dominik Stadler wrote:
> > >> Hi,
> > >>
> > >> Sorry for replying late here, not sure if you already did these changes,
> > >> but the code in CleanerUtil tries to handle this gracefully with not
> > >> cleaning on JDKs which do not support this.
> > >>
> > >> There should be no compile-time dependency on any unsafe object, during
> > >> runtime we try to get a cleaner and simply not unmap buffers cleanly if
> not
> > >> possible for some reason.
> > >>
> > >> Which new problem do you see with this approach when using the JDK
> > module
> > >> system?
> > >>
> > >> Thanks... Dominik.
> > >>
> > >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> > <[hidden email]>
> > >> wrote:
> > >>
> > >>> Hi *,
> > >>>
> > >>> I'm facing the same problem as described in
> > >>> https://issues.apache.org/jira/browse/LUCENE-6989
> > >>>
> > >>> Is it ok for you, if I get our build more or less to run with the module
> > >>> path (instead of the classpath) when running in a JDK 9+ and later try to
> > >>> fix the above Cleaner problem?
> > >>>
> > >>> I simply would like to focus on one issue now and as we a have
> > >>> multi-release jars after my commit, a JDK dependent solution shouldn't
> be
> > a
> > >>> problem anymore.
> > >>>
> > >>> Andi
> > >>>
> > >>>
> > >>>
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler-2
Ah I think I know!The code is fine, it's just how you use it.

Could it be that you are compiling this as a module and also use it as a module (the stack trace implies this)? If you use the code as a "real" module, you have to import explicitely in your module-info the "jdk.unsupported" module, otherwise you won't get access to sun.misc. You may need to add this to the auto-module descriptor or module-info.java.

This must not only be done in tests, Your JAR file MUST declare that it needs "jdk.unsupported" module. If it does not do this CleanerUtil is useless. In contrast to the classpath approach, where there is a default set of modules enabled, in a modulepath you only get what you declare - very simple.

In short:
You code fails, because the Class.forName() in the Java 9 code will not find "sun.misc.Unsafe", because its shielded by the module system. Because of this it falls back to the Java 8 code, which won't work at all (causing the exceptions).

Uwe

-----
Uwe Schindler
[hidden email]
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
https://lucene.apache.org/

> -----Original Message-----
> From: Uwe Schindler <[hidden email]>
> Sent: Friday, August 7, 2020 5:20 PM
> To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> <[hidden email]>; [hidden email]
> Subject: RE: Java 9+ modules / cleaner
>
> Hi,
>
> sorry I did not read all, so you already use my code:
>
> The exception you see is coming from the fact that the first part for Java 9 does
> not work at all. It looks like the code uses Java 8 variant (see line number in
> exception) instead of the Java 9 variant.
>
> Can you make a breakpoint here:
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> anerUtil.java?view=markup#l99
>
> And step through the code? It might fall through to the catch block and java 8
> code.
>
> Uwe
>
> -----
> Uwe Schindler
> Achterdiek 19, D-28357 Bremen
> https://www.thetaphi.de
> eMail: [hidden email]
>
> > -----Original Message-----
> > From: Uwe Schindler <[hidden email]>
> > Sent: Friday, August 7, 2020 5:06 PM
> > To: 'Andreas Beeker' <[hidden email]>; [hidden email]
> > Cc: [hidden email]
> > Subject: RE: Java 9+ modules / cleaner
> >
> > Hi,
> >
> > > I currently try to understand, how to call the Cleaner in Java 14 (or 9+)
> > without
> > > adding the --add-opens JVM options.
> >
> > Yeah, your code won't work correctly with Java 9 at all. You may fix it with
> > some opens, but still types of internal calsses changed, so its just risky
> > (everything is subject to change).
> >
> > > As you've worked on this in LUCENE-6989, you might have a few hints for
> me.
> >
> > I think you can more or less copy the code from Lucene (use the branch_8x
> > version, as Master requires Java 11, so has no Java 8 code anymore, see here:
> > https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use
> Reflection
> > at all, it works with Method Handles. The important thing is: Method handles
> > are linked earyl, once you have built the final method handle to invoke the
> > cleaner, you can call it without requesting any additional right (security
> > manager). In addition you know beforehand if it works at all (it cannot throw
> > extra reflective exceptions).
> >
> > The main change in Java 9 is (and this is what's officially "supported" by
> > OpenJDK developer): They added a new method to sun.misc.Unsafe (the
> legacy
> > one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base and is
> > open to public (if you know how to get the singleton) for the time being. To
> get
> > the singleton, you need reflection and the code must allow to do
> setAccessible
> > on the getter, but once you have it, it's useable.
> >
> > To unmap a Bytebuffer, Lucene creates a MethodHandle with some signature
> > like "unmap(ByteBuffer b)" complete pre-configued on startup of class
> > depending on Java version:
> > - In Java 9 and above it uses reflection to get the Unsafe instance (this
> requires
> > security manager to allow it). Then it looks up the method
> > "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the final
> > methodhandle is casted to me "void unmap(ByteBuffer)"
> > - In Java 8 and before it uses more or less the old approach by checking the
> > private method to get the Cleaner instance. Finally it calls cleaner.clean().
> This
> > can also be composed to a MethodHandle with exactly same signature (using
> > the famous MethodHandle transformation and bindings, introducing some
> null
> > checks). The result is also a methodhandle with signature "void
> > unmap(ByteBuffer)".
> >
> > Once all this is done, the methodhandle with platform independent signature
> > can be called without any exception handling from any code, so be sure to
> keep
> > it safe in private final fields fully internal to your implementaion (otherwise
> it's
> > a security issue).
> >
> > > I've checked the Lucene implementation, but that code is similar to POIs
> > > current implementation. [1]
> > > As I don't see the Runable interface, I might look at the wrong branch.
> >
> > That won't work. It's the same approach like the old one, just with other class
> > types. You cant work around the internal Cleaner interface. This is why
> > sun.misc.Unsafe#invokeCleaner() was added.
> >
> > > Any ideas?
> >
> > See above. I'd copy the code from Lucene: It's early binding and failsafe, once
> > you got the MethodHandle. The approach should work with Java 7+. In Java 7
> > there is one additional helper method needed for the methodHandle
> regarding
> > the null check!
> >
> > Java 8 is here: https://tinyurl.com/y47euqfg
> > Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k (this
> > adds a "compatibility method" to replace missing "Objects#nonNull(Object)",
> > but it's identical otherwise; we added this to make Lucene 5.5 still compatible
> > with Java 9, long after support ended, because customers need this).
> >
> > Uwe
> >
> > > Best wishes,
> > > Andi
> > >
> > >
> > > [1]
> > > https://github.com/apache/lucene-
> > >
> >
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> > > ry.java#L338
> > > vs.
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > anerUtil.java?view=markup#l91
> > >
> > > On 09.07.20 00:34, Andreas Beeker wrote:
> > > > Hi Dominik,
> > > >
> > > > the goal is to have no --add-opens or similar jvm arguments. In this case
> we
> > > get the following exception:
> > > >
> > > >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make
> > public
> > > jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible:
> module
> > > java.base does not "opens java.nio" to module org.apache.poi.poi
> > > >     [junit]     at
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > bject.java:349)
> > > >     [junit]     at
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > bject.java:289)
> > > >     [junit]     at
> > >
> java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> > > >     [junit]     at
> > > java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> > > rUtil.java:116)
> > > >     [junit]     at
> > >
> >
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> > > )
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> > > a:77)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> > > ackedDataSource.java:189)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> > > e$0(FileBackedDataSource.java:162)
> > > >     [junit]     at
> > > java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> > > kedDataSource.java:162)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> > > FileSystem.java:764)
> > > >     [junit]     at
> > >
> >
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> > > Write.java:539)
> > > >
> > > > If we use the Runnable aproach mentioned in the lucene bug, this should
> > > work again.
> > > >
> > > > In the meantime I had to move a few sources from ooxml to main
> > > (crypto.agile) and we need to find a way, e.g. using multi release classes, for
> > > our WorkbookFactory, as with JIgsaw the poi main module can't reflect into
> > > ooxml anymore. This was also a reason for moving the agile crypto into poi
> > > main and removing the use XmlBeans schemas there.
> > > >
> > > > Best wishes,
> > > > Andi
> > > >
> > > >
> > > > On 07.07.20 18:00, Dominik Stadler wrote:
> > > >> Hi,
> > > >>
> > > >> Sorry for replying late here, not sure if you already did these changes,
> > > >> but the code in CleanerUtil tries to handle this gracefully with not
> > > >> cleaning on JDKs which do not support this.
> > > >>
> > > >> There should be no compile-time dependency on any unsafe object,
> during
> > > >> runtime we try to get a cleaner and simply not unmap buffers cleanly if
> > not
> > > >> possible for some reason.
> > > >>
> > > >> Which new problem do you see with this approach when using the JDK
> > > module
> > > >> system?
> > > >>
> > > >> Thanks... Dominik.
> > > >>
> > > >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> > > <[hidden email]>
> > > >> wrote:
> > > >>
> > > >>> Hi *,
> > > >>>
> > > >>> I'm facing the same problem as described in
> > > >>> https://issues.apache.org/jira/browse/LUCENE-6989
> > > >>>
> > > >>> Is it ok for you, if I get our build more or less to run with the module
> > > >>> path (instead of the classpath) when running in a JDK 9+ and later try to
> > > >>> fix the above Cleaner problem?
> > > >>>
> > > >>> I simply would like to focus on one issue now and as we a have
> > > >>> multi-release jars after my commit, a JDK dependent solution shouldn't
> > be
> > > a
> > > >>> problem anymore.
> > > >>>
> > > >>> Andi
> > > >>>
> > > >>>
> > > >>>
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler-2
To confirm: If you use POI as a JAR on classpath it will work without modifications: Try it out.
For the module system you need to import jdk.unsupported module.

This is hard to figure out, the stack trace gave me the hint. It’s a HUGE difference if you put a JAR file on classpath (there it works) or on modulepath (won't work without importing jdk.unsupported).

In both cases, if everything is declared correctly in JAR file no exports/imports or command line options are needed.

Lucene does not support Modules at all, so it's no issue for us.

Uwe

-----
Uwe Schindler
[hidden email]
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
https://lucene.apache.org/

> -----Original Message-----
> From: Uwe Schindler <[hidden email]>
> Sent: Friday, August 7, 2020 6:12 PM
> To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> <[hidden email]>
> Subject: RE: Java 9+ modules / cleaner
>
> Ah I think I know!The code is fine, it's just how you use it.
>
> Could it be that you are compiling this as a module and also use it as a module
> (the stack trace implies this)? If you use the code as a "real" module, you have
> to import explicitely in your module-info the "jdk.unsupported" module,
> otherwise you won't get access to sun.misc. You may need to add this to the
> auto-module descriptor or module-info.java.
>
> This must not only be done in tests, Your JAR file MUST declare that it needs
> "jdk.unsupported" module. If it does not do this CleanerUtil is useless. In
> contrast to the classpath approach, where there is a default set of modules
> enabled, in a modulepath you only get what you declare - very simple.
>
> In short:
> You code fails, because the Class.forName() in the Java 9 code will not find
> "sun.misc.Unsafe", because its shielded by the module system. Because of this
> it falls back to the Java 8 code, which won't work at all (causing the
> exceptions).
>
> Uwe
>
> -----
> Uwe Schindler
> [hidden email]
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> https://lucene.apache.org/
>
> > -----Original Message-----
> > From: Uwe Schindler <[hidden email]>
> > Sent: Friday, August 7, 2020 5:20 PM
> > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > <[hidden email]>; [hidden email]
> > Subject: RE: Java 9+ modules / cleaner
> >
> > Hi,
> >
> > sorry I did not read all, so you already use my code:
> >
> > The exception you see is coming from the fact that the first part for Java 9
> does
> > not work at all. It looks like the code uses Java 8 variant (see line number in
> > exception) instead of the Java 9 variant.
> >
> > Can you make a breakpoint here:
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > anerUtil.java?view=markup#l99
> >
> > And step through the code? It might fall through to the catch block and java 8
> > code.
> >
> > Uwe
> >
> > -----
> > Uwe Schindler
> > Achterdiek 19, D-28357 Bremen
> > https://www.thetaphi.de
> > eMail: [hidden email]
> >
> > > -----Original Message-----
> > > From: Uwe Schindler <[hidden email]>
> > > Sent: Friday, August 7, 2020 5:06 PM
> > > To: 'Andreas Beeker' <[hidden email]>; [hidden email]
> > > Cc: [hidden email]
> > > Subject: RE: Java 9+ modules / cleaner
> > >
> > > Hi,
> > >
> > > > I currently try to understand, how to call the Cleaner in Java 14 (or 9+)
> > > without
> > > > adding the --add-opens JVM options.
> > >
> > > Yeah, your code won't work correctly with Java 9 at all. You may fix it with
> > > some opens, but still types of internal calsses changed, so its just risky
> > > (everything is subject to change).
> > >
> > > > As you've worked on this in LUCENE-6989, you might have a few hints for
> > me.
> > >
> > > I think you can more or less copy the code from Lucene (use the branch_8x
> > > version, as Master requires Java 11, so has no Java 8 code anymore, see
> here:
> > > https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use
> > Reflection
> > > at all, it works with Method Handles. The important thing is: Method
> handles
> > > are linked earyl, once you have built the final method handle to invoke the
> > > cleaner, you can call it without requesting any additional right (security
> > > manager). In addition you know beforehand if it works at all (it cannot
> throw
> > > extra reflective exceptions).
> > >
> > > The main change in Java 9 is (and this is what's officially "supported" by
> > > OpenJDK developer): They added a new method to sun.misc.Unsafe (the
> > legacy
> > > one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base and is
> > > open to public (if you know how to get the singleton) for the time being. To
> > get
> > > the singleton, you need reflection and the code must allow to do
> > setAccessible
> > > on the getter, but once you have it, it's useable.
> > >
> > > To unmap a Bytebuffer, Lucene creates a MethodHandle with some
> signature
> > > like "unmap(ByteBuffer b)" complete pre-configued on startup of class
> > > depending on Java version:
> > > - In Java 9 and above it uses reflection to get the Unsafe instance (this
> > requires
> > > security manager to allow it). Then it looks up the method
> > > "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the final
> > > methodhandle is casted to me "void unmap(ByteBuffer)"
> > > - In Java 8 and before it uses more or less the old approach by checking the
> > > private method to get the Cleaner instance. Finally it calls cleaner.clean().
> > This
> > > can also be composed to a MethodHandle with exactly same signature
> (using
> > > the famous MethodHandle transformation and bindings, introducing some
> > null
> > > checks). The result is also a methodhandle with signature "void
> > > unmap(ByteBuffer)".
> > >
> > > Once all this is done, the methodhandle with platform independent
> signature
> > > can be called without any exception handling from any code, so be sure to
> > keep
> > > it safe in private final fields fully internal to your implementaion (otherwise
> > it's
> > > a security issue).
> > >
> > > > I've checked the Lucene implementation, but that code is similar to POIs
> > > > current implementation. [1]
> > > > As I don't see the Runable interface, I might look at the wrong branch.
> > >
> > > That won't work. It's the same approach like the old one, just with other
> class
> > > types. You cant work around the internal Cleaner interface. This is why
> > > sun.misc.Unsafe#invokeCleaner() was added.
> > >
> > > > Any ideas?
> > >
> > > See above. I'd copy the code from Lucene: It's early binding and failsafe,
> once
> > > you got the MethodHandle. The approach should work with Java 7+. In Java
> 7
> > > there is one additional helper method needed for the methodHandle
> > regarding
> > > the null check!
> > >
> > > Java 8 is here: https://tinyurl.com/y47euqfg
> > > Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k
> (this
> > > adds a "compatibility method" to replace missing
> "Objects#nonNull(Object)",
> > > but it's identical otherwise; we added this to make Lucene 5.5 still
> compatible
> > > with Java 9, long after support ended, because customers need this).
> > >
> > > Uwe
> > >
> > > > Best wishes,
> > > > Andi
> > > >
> > > >
> > > > [1]
> > > > https://github.com/apache/lucene-
> > > >
> > >
> >
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> > > > ry.java#L338
> > > > vs.
> > > >
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > > anerUtil.java?view=markup#l91
> > > >
> > > > On 09.07.20 00:34, Andreas Beeker wrote:
> > > > > Hi Dominik,
> > > > >
> > > > > the goal is to have no --add-opens or similar jvm arguments. In this case
> > we
> > > > get the following exception:
> > > > >
> > > > >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make
> > > public
> > > > jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible:
> > module
> > > > java.base does not "opens java.nio" to module org.apache.poi.poi
> > > > >     [junit]     at
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > bject.java:349)
> > > > >     [junit]     at
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > bject.java:289)
> > > > >     [junit]     at
> > > >
> > java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> > > > >     [junit]     at
> > > > java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> > > > rUtil.java:116)
> > > > >     [junit]     at
> > > >
> > >
> >
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> > > > )
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> > > > a:77)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> > > > ackedDataSource.java:189)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> > > > e$0(FileBackedDataSource.java:162)
> > > > >     [junit]     at
> > > > java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> > > > kedDataSource.java:162)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> > > > FileSystem.java:764)
> > > > >     [junit]     at
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> > > > Write.java:539)
> > > > >
> > > > > If we use the Runnable aproach mentioned in the lucene bug, this
> should
> > > > work again.
> > > > >
> > > > > In the meantime I had to move a few sources from ooxml to main
> > > > (crypto.agile) and we need to find a way, e.g. using multi release classes,
> for
> > > > our WorkbookFactory, as with JIgsaw the poi main module can't reflect
> into
> > > > ooxml anymore. This was also a reason for moving the agile crypto into
> poi
> > > > main and removing the use XmlBeans schemas there.
> > > > >
> > > > > Best wishes,
> > > > > Andi
> > > > >
> > > > >
> > > > > On 07.07.20 18:00, Dominik Stadler wrote:
> > > > >> Hi,
> > > > >>
> > > > >> Sorry for replying late here, not sure if you already did these changes,
> > > > >> but the code in CleanerUtil tries to handle this gracefully with not
> > > > >> cleaning on JDKs which do not support this.
> > > > >>
> > > > >> There should be no compile-time dependency on any unsafe object,
> > during
> > > > >> runtime we try to get a cleaner and simply not unmap buffers cleanly if
> > > not
> > > > >> possible for some reason.
> > > > >>
> > > > >> Which new problem do you see with this approach when using the JDK
> > > > module
> > > > >> system?
> > > > >>
> > > > >> Thanks... Dominik.
> > > > >>
> > > > >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> > > > <[hidden email]>
> > > > >> wrote:
> > > > >>
> > > > >>> Hi *,
> > > > >>>
> > > > >>> I'm facing the same problem as described in
> > > > >>> https://issues.apache.org/jira/browse/LUCENE-6989
> > > > >>>
> > > > >>> Is it ok for you, if I get our build more or less to run with the module
> > > > >>> path (instead of the classpath) when running in a JDK 9+ and later try
> to
> > > > >>> fix the above Cleaner problem?
> > > > >>>
> > > > >>> I simply would like to focus on one issue now and as we a have
> > > > >>> multi-release jars after my commit, a JDK dependent solution
> shouldn't
> > > be
> > > > a
> > > > >>> problem anymore.
> > > > >>>
> > > > >>> Andi
> > > > >>>
> > > > >>>
> > > > >>>
> > > > >
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > > > >
> > > >
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler-2
In reply to this post by Uwe Schindler-2
See also here:
https://blogs.oracle.com/java/get-ready-jdk9

Sorry for not understanding the whole problem (module system usage instaed of classpath). Your code is perfectly valid and works with Java 9, it's just broken

I still wonder why you get this Exception at all. It should be swallowed by the last try/catch block and saved as a warning for "UNMAP_UNSUPPORTED". Or did you hack the code to get the full stack trace?

Uwe

-----
Uwe Schindler
[hidden email]
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
https://lucene.apache.org/

> -----Original Message-----
> From: Uwe Schindler <[hidden email]>
> Sent: Friday, August 7, 2020 6:16 PM
> To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> <[hidden email]>
> Subject: RE: Java 9+ modules / cleaner
>
> To confirm: If you use POI as a JAR on classpath it will work without
> modifications: Try it out.
> For the module system you need to import jdk.unsupported module.
>
> This is hard to figure out, the stack trace gave me the hint. It’s a HUGE
> difference if you put a JAR file on classpath (there it works) or on modulepath
> (won't work without importing jdk.unsupported).
>
> In both cases, if everything is declared correctly in JAR file no exports/imports
> or command line options are needed.
>
> Lucene does not support Modules at all, so it's no issue for us.
>
> Uwe
>
> -----
> Uwe Schindler
> [hidden email]
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> https://lucene.apache.org/
>
> > -----Original Message-----
> > From: Uwe Schindler <[hidden email]>
> > Sent: Friday, August 7, 2020 6:12 PM
> > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > <[hidden email]>
> > Subject: RE: Java 9+ modules / cleaner
> >
> > Ah I think I know!The code is fine, it's just how you use it.
> >
> > Could it be that you are compiling this as a module and also use it as a
> module
> > (the stack trace implies this)? If you use the code as a "real" module, you have
> > to import explicitely in your module-info the "jdk.unsupported" module,
> > otherwise you won't get access to sun.misc. You may need to add this to the
> > auto-module descriptor or module-info.java.
> >
> > This must not only be done in tests, Your JAR file MUST declare that it needs
> > "jdk.unsupported" module. If it does not do this CleanerUtil is useless. In
> > contrast to the classpath approach, where there is a default set of modules
> > enabled, in a modulepath you only get what you declare - very simple.
> >
> > In short:
> > You code fails, because the Class.forName() in the Java 9 code will not find
> > "sun.misc.Unsafe", because its shielded by the module system. Because of
> this
> > it falls back to the Java 8 code, which won't work at all (causing the
> > exceptions).
> >
> > Uwe
> >
> > -----
> > Uwe Schindler
> > [hidden email]
> > ASF Member, Apache Lucene PMC / Committer
> > Bremen, Germany
> > https://lucene.apache.org/
> >
> > > -----Original Message-----
> > > From: Uwe Schindler <[hidden email]>
> > > Sent: Friday, August 7, 2020 5:20 PM
> > > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > > <[hidden email]>; [hidden email]
> > > Subject: RE: Java 9+ modules / cleaner
> > >
> > > Hi,
> > >
> > > sorry I did not read all, so you already use my code:
> > >
> > > The exception you see is coming from the fact that the first part for Java 9
> > does
> > > not work at all. It looks like the code uses Java 8 variant (see line number in
> > > exception) instead of the Java 9 variant.
> > >
> > > Can you make a breakpoint here:
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > anerUtil.java?view=markup#l99
> > >
> > > And step through the code? It might fall through to the catch block and java
> 8
> > > code.
> > >
> > > Uwe
> > >
> > > -----
> > > Uwe Schindler
> > > Achterdiek 19, D-28357 Bremen
> > > https://www.thetaphi.de
> > > eMail: [hidden email]
> > >
> > > > -----Original Message-----
> > > > From: Uwe Schindler <[hidden email]>
> > > > Sent: Friday, August 7, 2020 5:06 PM
> > > > To: 'Andreas Beeker' <[hidden email]>; [hidden email]
> > > > Cc: [hidden email]
> > > > Subject: RE: Java 9+ modules / cleaner
> > > >
> > > > Hi,
> > > >
> > > > > I currently try to understand, how to call the Cleaner in Java 14 (or 9+)
> > > > without
> > > > > adding the --add-opens JVM options.
> > > >
> > > > Yeah, your code won't work correctly with Java 9 at all. You may fix it with
> > > > some opens, but still types of internal calsses changed, so its just risky
> > > > (everything is subject to change).
> > > >
> > > > > As you've worked on this in LUCENE-6989, you might have a few hints
> for
> > > me.
> > > >
> > > > I think you can more or less copy the code from Lucene (use the
> branch_8x
> > > > version, as Master requires Java 11, so has no Java 8 code anymore, see
> > here:
> > > > https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use
> > > Reflection
> > > > at all, it works with Method Handles. The important thing is: Method
> > handles
> > > > are linked earyl, once you have built the final method handle to invoke
> the
> > > > cleaner, you can call it without requesting any additional right (security
> > > > manager). In addition you know beforehand if it works at all (it cannot
> > throw
> > > > extra reflective exceptions).
> > > >
> > > > The main change in Java 9 is (and this is what's officially "supported" by
> > > > OpenJDK developer): They added a new method to sun.misc.Unsafe (the
> > > legacy
> > > > one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base and
> is
> > > > open to public (if you know how to get the singleton) for the time being.
> To
> > > get
> > > > the singleton, you need reflection and the code must allow to do
> > > setAccessible
> > > > on the getter, but once you have it, it's useable.
> > > >
> > > > To unmap a Bytebuffer, Lucene creates a MethodHandle with some
> > signature
> > > > like "unmap(ByteBuffer b)" complete pre-configued on startup of class
> > > > depending on Java version:
> > > > - In Java 9 and above it uses reflection to get the Unsafe instance (this
> > > requires
> > > > security manager to allow it). Then it looks up the method
> > > > "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the final
> > > > methodhandle is casted to me "void unmap(ByteBuffer)"
> > > > - In Java 8 and before it uses more or less the old approach by checking
> the
> > > > private method to get the Cleaner instance. Finally it calls cleaner.clean().
> > > This
> > > > can also be composed to a MethodHandle with exactly same signature
> > (using
> > > > the famous MethodHandle transformation and bindings, introducing
> some
> > > null
> > > > checks). The result is also a methodhandle with signature "void
> > > > unmap(ByteBuffer)".
> > > >
> > > > Once all this is done, the methodhandle with platform independent
> > signature
> > > > can be called without any exception handling from any code, so be sure to
> > > keep
> > > > it safe in private final fields fully internal to your implementaion
> (otherwise
> > > it's
> > > > a security issue).
> > > >
> > > > > I've checked the Lucene implementation, but that code is similar to POIs
> > > > > current implementation. [1]
> > > > > As I don't see the Runable interface, I might look at the wrong branch.
> > > >
> > > > That won't work. It's the same approach like the old one, just with other
> > class
> > > > types. You cant work around the internal Cleaner interface. This is why
> > > > sun.misc.Unsafe#invokeCleaner() was added.
> > > >
> > > > > Any ideas?
> > > >
> > > > See above. I'd copy the code from Lucene: It's early binding and failsafe,
> > once
> > > > you got the MethodHandle. The approach should work with Java 7+. In
> Java
> > 7
> > > > there is one additional helper method needed for the methodHandle
> > > regarding
> > > > the null check!
> > > >
> > > > Java 8 is here: https://tinyurl.com/y47euqfg
> > > > Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k
> > (this
> > > > adds a "compatibility method" to replace missing
> > "Objects#nonNull(Object)",
> > > > but it's identical otherwise; we added this to make Lucene 5.5 still
> > compatible
> > > > with Java 9, long after support ended, because customers need this).
> > > >
> > > > Uwe
> > > >
> > > > > Best wishes,
> > > > > Andi
> > > > >
> > > > >
> > > > > [1]
> > > > > https://github.com/apache/lucene-
> > > > >
> > > >
> > >
> >
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> > > > > ry.java#L338
> > > > > vs.
> > > > >
> > > >
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > > > anerUtil.java?view=markup#l91
> > > > >
> > > > > On 09.07.20 00:34, Andreas Beeker wrote:
> > > > > > Hi Dominik,
> > > > > >
> > > > > > the goal is to have no --add-opens or similar jvm arguments. In this
> case
> > > we
> > > > > get the following exception:
> > > > > >
> > > > > >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to make
> > > > public
> > > > > jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible:
> > > module
> > > > > java.base does not "opens java.nio" to module org.apache.poi.poi
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > > bject.java:349)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > > bject.java:289)
> > > > > >     [junit]     at
> > > > >
> > >
> java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> > > > > >     [junit]     at
> > > > > java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> > > > > rUtil.java:116)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> > > > > )
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> > > > > a:77)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> > > > > ackedDataSource.java:189)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> > > > > e$0(FileBackedDataSource.java:162)
> > > > > >     [junit]     at
> > > > >
> java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> > > > > kedDataSource.java:162)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> > > > > FileSystem.java:764)
> > > > > >     [junit]     at
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> > > > > Write.java:539)
> > > > > >
> > > > > > If we use the Runnable aproach mentioned in the lucene bug, this
> > should
> > > > > work again.
> > > > > >
> > > > > > In the meantime I had to move a few sources from ooxml to main
> > > > > (crypto.agile) and we need to find a way, e.g. using multi release classes,
> > for
> > > > > our WorkbookFactory, as with JIgsaw the poi main module can't reflect
> > into
> > > > > ooxml anymore. This was also a reason for moving the agile crypto into
> > poi
> > > > > main and removing the use XmlBeans schemas there.
> > > > > >
> > > > > > Best wishes,
> > > > > > Andi
> > > > > >
> > > > > >
> > > > > > On 07.07.20 18:00, Dominik Stadler wrote:
> > > > > >> Hi,
> > > > > >>
> > > > > >> Sorry for replying late here, not sure if you already did these changes,
> > > > > >> but the code in CleanerUtil tries to handle this gracefully with not
> > > > > >> cleaning on JDKs which do not support this.
> > > > > >>
> > > > > >> There should be no compile-time dependency on any unsafe object,
> > > during
> > > > > >> runtime we try to get a cleaner and simply not unmap buffers cleanly
> if
> > > > not
> > > > > >> possible for some reason.
> > > > > >>
> > > > > >> Which new problem do you see with this approach when using the
> JDK
> > > > > module
> > > > > >> system?
> > > > > >>
> > > > > >> Thanks... Dominik.
> > > > > >>
> > > > > >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> > > > > <[hidden email]>
> > > > > >> wrote:
> > > > > >>
> > > > > >>> Hi *,
> > > > > >>>
> > > > > >>> I'm facing the same problem as described in
> > > > > >>> https://issues.apache.org/jira/browse/LUCENE-6989
> > > > > >>>
> > > > > >>> Is it ok for you, if I get our build more or less to run with the module
> > > > > >>> path (instead of the classpath) when running in a JDK 9+ and later
> try
> > to
> > > > > >>> fix the above Cleaner problem?
> > > > > >>>
> > > > > >>> I simply would like to focus on one issue now and as we a have
> > > > > >>> multi-release jars after my commit, a JDK dependent solution
> > shouldn't
> > > > be
> > > > > a
> > > > > >>> problem anymore.
> > > > > >>>
> > > > > >>> Andi
> > > > > >>>
> > > > > >>>
> > > > > >>>
> > > > > >
> > > > > >
> > > > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: [hidden email]
> > > > > > For additional commands, e-mail: [hidden email]
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: Java 9+ modules / cleaner

Uwe Schindler-2
> Sorry for not understanding the whole problem (module system usage instaed
> of classpath). Your code is perfectly valid and works with Java 9, it's just broken

...with module system as its not fully declared all imports. The compile won't find the issue, as it's in dynamic linking code.

> -----
> Uwe Schindler
> [hidden email]
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> https://lucene.apache.org/
>
> > -----Original Message-----
> > From: Uwe Schindler <[hidden email]>
> > Sent: Friday, August 7, 2020 6:16 PM
> > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > <[hidden email]>
> > Subject: RE: Java 9+ modules / cleaner
> >
> > To confirm: If you use POI as a JAR on classpath it will work without
> > modifications: Try it out.
> > For the module system you need to import jdk.unsupported module.
> >
> > This is hard to figure out, the stack trace gave me the hint. It’s a HUGE
> > difference if you put a JAR file on classpath (there it works) or on modulepath
> > (won't work without importing jdk.unsupported).
> >
> > In both cases, if everything is declared correctly in JAR file no exports/imports
> > or command line options are needed.
> >
> > Lucene does not support Modules at all, so it's no issue for us.
> >
> > Uwe
> >
> > -----
> > Uwe Schindler
> > [hidden email]
> > ASF Member, Apache Lucene PMC / Committer
> > Bremen, Germany
> > https://lucene.apache.org/
> >
> > > -----Original Message-----
> > > From: Uwe Schindler <[hidden email]>
> > > Sent: Friday, August 7, 2020 6:12 PM
> > > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > > <[hidden email]>
> > > Subject: RE: Java 9+ modules / cleaner
> > >
> > > Ah I think I know!The code is fine, it's just how you use it.
> > >
> > > Could it be that you are compiling this as a module and also use it as a
> > module
> > > (the stack trace implies this)? If you use the code as a "real" module, you
> have
> > > to import explicitely in your module-info the "jdk.unsupported" module,
> > > otherwise you won't get access to sun.misc. You may need to add this to the
> > > auto-module descriptor or module-info.java.
> > >
> > > This must not only be done in tests, Your JAR file MUST declare that it
> needs
> > > "jdk.unsupported" module. If it does not do this CleanerUtil is useless. In
> > > contrast to the classpath approach, where there is a default set of modules
> > > enabled, in a modulepath you only get what you declare - very simple.
> > >
> > > In short:
> > > You code fails, because the Class.forName() in the Java 9 code will not find
> > > "sun.misc.Unsafe", because its shielded by the module system. Because of
> > this
> > > it falls back to the Java 8 code, which won't work at all (causing the
> > > exceptions).
> > >
> > > Uwe
> > >
> > > -----
> > > Uwe Schindler
> > > [hidden email]
> > > ASF Member, Apache Lucene PMC / Committer
> > > Bremen, Germany
> > > https://lucene.apache.org/
> > >
> > > > -----Original Message-----
> > > > From: Uwe Schindler <[hidden email]>
> > > > Sent: Friday, August 7, 2020 5:20 PM
> > > > To: 'POI Developers List' <[hidden email]>; 'Andreas Beeker'
> > > > <[hidden email]>; [hidden email]
> > > > Subject: RE: Java 9+ modules / cleaner
> > > >
> > > > Hi,
> > > >
> > > > sorry I did not read all, so you already use my code:
> > > >
> > > > The exception you see is coming from the fact that the first part for Java 9
> > > does
> > > > not work at all. It looks like the code uses Java 8 variant (see line number
> in
> > > > exception) instead of the Java 9 variant.
> > > >
> > > > Can you make a breakpoint here:
> > > >
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > > anerUtil.java?view=markup#l99
> > > >
> > > > And step through the code? It might fall through to the catch block and
> java
> > 8
> > > > code.
> > > >
> > > > Uwe
> > > >
> > > > -----
> > > > Uwe Schindler
> > > > Achterdiek 19, D-28357 Bremen
> > > > https://www.thetaphi.de
> > > > eMail: [hidden email]
> > > >
> > > > > -----Original Message-----
> > > > > From: Uwe Schindler <[hidden email]>
> > > > > Sent: Friday, August 7, 2020 5:06 PM
> > > > > To: 'Andreas Beeker' <[hidden email]>; [hidden email]
> > > > > Cc: [hidden email]
> > > > > Subject: RE: Java 9+ modules / cleaner
> > > > >
> > > > > Hi,
> > > > >
> > > > > > I currently try to understand, how to call the Cleaner in Java 14 (or 9+)
> > > > > without
> > > > > > adding the --add-opens JVM options.
> > > > >
> > > > > Yeah, your code won't work correctly with Java 9 at all. You may fix it
> with
> > > > > some opens, but still types of internal calsses changed, so its just risky
> > > > > (everything is subject to change).
> > > > >
> > > > > > As you've worked on this in LUCENE-6989, you might have a few hints
> > for
> > > > me.
> > > > >
> > > > > I think you can more or less copy the code from Lucene (use the
> > branch_8x
> > > > > version, as Master requires Java 11, so has no Java 8 code anymore, see
> > > here:
> > > > > https://tinyurl.com/y47euqfg). The aproach in Lucene does NOT use
> > > > Reflection
> > > > > at all, it works with Method Handles. The important thing is: Method
> > > handles
> > > > > are linked earyl, once you have built the final method handle to invoke
> > the
> > > > > cleaner, you can call it without requesting any additional right (security
> > > > > manager). In addition you know beforehand if it works at all (it cannot
> > > throw
> > > > > extra reflective exceptions).
> > > > >
> > > > > The main change in Java 9 is (and this is what's officially "supported" by
> > > > > OpenJDK developer): They added a new method to sun.misc.Unsafe (the
> > > > legacy
> > > > > one), Unsafe#invokeCleaner(ByteBuffer). This class is still in java.base
> and
> > is
> > > > > open to public (if you know how to get the singleton) for the time being.
> > To
> > > > get
> > > > > the singleton, you need reflection and the code must allow to do
> > > > setAccessible
> > > > > on the getter, but once you have it, it's useable.
> > > > >
> > > > > To unmap a Bytebuffer, Lucene creates a MethodHandle with some
> > > signature
> > > > > like "unmap(ByteBuffer b)" complete pre-configued on startup of class
> > > > > depending on Java version:
> > > > > - In Java 9 and above it uses reflection to get the Unsafe instance (this
> > > > requires
> > > > > security manager to allow it). Then it looks up the method
> > > > > "invokeCleaner(ByteBuffer) and binds it to the unsafe  singleton, the
> final
> > > > > methodhandle is casted to me "void unmap(ByteBuffer)"
> > > > > - In Java 8 and before it uses more or less the old approach by checking
> > the
> > > > > private method to get the Cleaner instance. Finally it calls
> cleaner.clean().
> > > > This
> > > > > can also be composed to a MethodHandle with exactly same signature
> > > (using
> > > > > the famous MethodHandle transformation and bindings, introducing
> > some
> > > > null
> > > > > checks). The result is also a methodhandle with signature "void
> > > > > unmap(ByteBuffer)".
> > > > >
> > > > > Once all this is done, the methodhandle with platform independent
> > > signature
> > > > > can be called without any exception handling from any code, so be sure
> to
> > > > keep
> > > > > it safe in private final fields fully internal to your implementaion
> > (otherwise
> > > > it's
> > > > > a security issue).
> > > > >
> > > > > > I've checked the Lucene implementation, but that code is similar to
> POIs
> > > > > > current implementation. [1]
> > > > > > As I don't see the Runable interface, I might look at the wrong branch.
> > > > >
> > > > > That won't work. It's the same approach like the old one, just with other
> > > class
> > > > > types. You cant work around the internal Cleaner interface. This is why
> > > > > sun.misc.Unsafe#invokeCleaner() was added.
> > > > >
> > > > > > Any ideas?
> > > > >
> > > > > See above. I'd copy the code from Lucene: It's early binding and failsafe,
> > > once
> > > > > you got the MethodHandle. The approach should work with Java 7+. In
> > Java
> > > 7
> > > > > there is one additional helper method needed for the methodHandle
> > > > regarding
> > > > > the null check!
> > > > >
> > > > > Java 8 is here: https://tinyurl.com/y47euqfg
> > > > > Hack for compatibility with Java 7 is here: https://tinyurl.com/y4drev3k
> > > (this
> > > > > adds a "compatibility method" to replace missing
> > > "Objects#nonNull(Object)",
> > > > > but it's identical otherwise; we added this to make Lucene 5.5 still
> > > compatible
> > > > > with Java 9, long after support ended, because customers need this).
> > > > >
> > > > > Uwe
> > > > >
> > > > > > Best wishes,
> > > > > > Andi
> > > > > >
> > > > > >
> > > > > > [1]
> > > > > > https://github.com/apache/lucene-
> > > > > >
> > > > >
> > > >
> > >
> >
> solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirecto
> > > > > > ry.java#L338
> > > > > > vs.
> > > > > >
> > > > >
> > > >
> > >
> >
> https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/Cle
> > > > > > anerUtil.java?view=markup#l91
> > > > > >
> > > > > > On 09.07.20 00:34, Andreas Beeker wrote:
> > > > > > > Hi Dominik,
> > > > > > >
> > > > > > > the goal is to have no --add-opens or similar jvm arguments. In this
> > case
> > > > we
> > > > > > get the following exception:
> > > > > > >
> > > > > > >    [junit] java.lang.reflect.InaccessibleObjectException: Unable to
> make
> > > > > public
> > > > > > jdk.internal.ref.Cleaner java.nio.DirectByteBuffer.cleaner() accessible:
> > > > module
> > > > > > java.base does not "opens java.nio" to module org.apache.poi.poi
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > > > bject.java:349)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleO
> > > > > > bject.java:289)
> > > > > > >     [junit]     at
> > > > > >
> > > >
> > java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:196)
> > > > > > >     [junit]     at
> > > > > > java.base/java.lang.reflect.Method.setAccessible(Method.java:190)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.unmapHackImpl(Cleane
> > > > > > rUtil.java:116)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> java.base/java.security.AccessController.doPrivileged(AccessController.java:312
> > > > > > )
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.CleanerUtil.<clinit>(CleanerUtil.jav
> > > > > > a:77)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.unmap(FileB
> > > > > > ackedDataSource.java:189)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.lambda$clos
> > > > > > e$0(FileBackedDataSource.java:162)
> > > > > > >     [junit]     at
> > > > > >
> > java.base/java.util.IdentityHashMap.forEach(IdentityHashMap.java:1356)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.nio.FileBackedDataSource.close(FileBac
> > > > > > kedDataSource.java:162)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.poifs.filesystem.POIFSFileSystem.close(POIFS
> > > > > > FileSystem.java:764)
> > > > > > >     [junit]     at
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.poi.poi/org.apache.poi.hpsf.basic.TestWrite.inPlacePOIFSWrite(Test
> > > > > > Write.java:539)
> > > > > > >
> > > > > > > If we use the Runnable aproach mentioned in the lucene bug, this
> > > should
> > > > > > work again.
> > > > > > >
> > > > > > > In the meantime I had to move a few sources from ooxml to main
> > > > > > (crypto.agile) and we need to find a way, e.g. using multi release
> classes,
> > > for
> > > > > > our WorkbookFactory, as with JIgsaw the poi main module can't
> reflect
> > > into
> > > > > > ooxml anymore. This was also a reason for moving the agile crypto
> into
> > > poi
> > > > > > main and removing the use XmlBeans schemas there.
> > > > > > >
> > > > > > > Best wishes,
> > > > > > > Andi
> > > > > > >
> > > > > > >
> > > > > > > On 07.07.20 18:00, Dominik Stadler wrote:
> > > > > > >> Hi,
> > > > > > >>
> > > > > > >> Sorry for replying late here, not sure if you already did these
> changes,
> > > > > > >> but the code in CleanerUtil tries to handle this gracefully with not
> > > > > > >> cleaning on JDKs which do not support this.
> > > > > > >>
> > > > > > >> There should be no compile-time dependency on any unsafe object,
> > > > during
> > > > > > >> runtime we try to get a cleaner and simply not unmap buffers
> cleanly
> > if
> > > > > not
> > > > > > >> possible for some reason.
> > > > > > >>
> > > > > > >> Which new problem do you see with this approach when using the
> > JDK
> > > > > > module
> > > > > > >> system?
> > > > > > >>
> > > > > > >> Thanks... Dominik.
> > > > > > >>
> > > > > > >> On Mon, Jun 29, 2020 at 10:36 PM Andreas Beeker
> > > > > > <[hidden email]>
> > > > > > >> wrote:
> > > > > > >>
> > > > > > >>> Hi *,
> > > > > > >>>
> > > > > > >>> I'm facing the same problem as described in
> > > > > > >>> https://issues.apache.org/jira/browse/LUCENE-6989
> > > > > > >>>
> > > > > > >>> Is it ok for you, if I get our build more or less to run with the
> module
> > > > > > >>> path (instead of the classpath) when running in a JDK 9+ and later
> > try
> > > to
> > > > > > >>> fix the above Cleaner problem?
> > > > > > >>>
> > > > > > >>> I simply would like to focus on one issue now and as we a have
> > > > > > >>> multi-release jars after my commit, a JDK dependent solution
> > > shouldn't
> > > > > be
> > > > > > a
> > > > > > >>> problem anymore.
> > > > > > >>>
> > > > > > >>> Andi
> > > > > > >>>
> > > > > > >>>
> > > > > > >>>
> > > > > > >
> > > > > > >
> > > > > > > ---------------------------------------------------------------------
> > > > > > > To unsubscribe, e-mail: [hidden email]
> > > > > > > For additional commands, e-mail: [hidden email]
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

kiwiwings
Thank you Uwe.

The "jdk.unsupported" module hint did the trick - at least the build is now getting over that error.

So our build uses classpath mode when run under Java 8 and modulepath for Java 9+ - actually everything except of the module-info is compiled in classpath mode and the module-info is then patched into the classes.
To release under Java 8, I compile/update the module-info.class to the source when compiling under Java 9+ - so this development noise is a bit of a drawback which we might solve by using timestamp checks in the build.

I thought about providing a multi-release version of the CleanerUtil, so we have only the Java 8 or the Java 9+ code active.
But then I'm puzzled how the JRE reacts, if you use classpath mode in Java 9+.
I guess it ignores the module-info modules and the patched multi-release class, effectively running the Java 8 code.
So we need both versions with that fallback mechanism.
TL;DR: don't change CleanerUtil.




On 07.08.20 18:24, Uwe Schindler wrote:
>> Sorry for not understanding the whole problem (module system usage instaed
>> of classpath). Your code is perfectly valid and works with Java 9, it's just broken
> ...with module system as its not fully declared all imports. The compile won't find the issue, as it's in dynamic linking code.
>



signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

Uwe Schindler-2
CleanerUtil dies not need to be multireleased. Just leave as is.

The problem was only the missing import of the unsupported module which is required for unmapping. If you are in module mode at runtime (module path) and the unsupported module is not there, the exception handler falls through to the Java 8 code. After that it complains that nothing works.

I still wonder where the exception came from. The code catches all of them and converts to a message which should get logged. So this is why I asked how you got the exception!

I would recommend to put the module-info.class into the multi release part of the jar (below meta-inf/versions/9).

Uwe

Am August 7, 2020 8:18:48 PM UTC schrieb Andreas Beeker <[hidden email]>:

>Thank you Uwe.
>
>The "jdk.unsupported" module hint did the trick - at least the build is
>now getting over that error.
>
>So our build uses classpath mode when run under Java 8 and modulepath
>for Java 9+ - actually everything except of the module-info is compiled
>in classpath mode and the module-info is then patched into the classes.
>To release under Java 8, I compile/update the module-info.class to the
>source when compiling under Java 9+ - so this development noise is a
>bit of a drawback which we might solve by using timestamp checks in the
>build.
>
>I thought about providing a multi-release version of the CleanerUtil,
>so we have only the Java 8 or the Java 9+ code active.
>But then I'm puzzled how the JRE reacts, if you use classpath mode in
>Java 9+.
>I guess it ignores the module-info modules and the patched
>multi-release class, effectively running the Java 8 code.
>So we need both versions with that fallback mechanism.
>TL;DR: don't change CleanerUtil.
>
>
>
>
>On 07.08.20 18:24, Uwe Schindler wrote:
>>> Sorry for not understanding the whole problem (module system usage
>instaed
>>> of classpath). Your code is perfectly valid and works with Java 9,
>it's just broken
>> ...with module system as its not fully declared all imports. The
>compile won't find the issue, as it's in dynamic linking code.
>>
Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

kiwiwings
> The problem was only the missing import of the unsupported module which is required for unmapping.
I think, I should also add the information about the permissions to a FAQ entry:
https://stackoverflow.com/a/54046774/2066598

> I still wonder where the exception came from.
Have a look at AccessibleObject:349 which prints the stacktrace to System.Err

> I would recommend to put the module-info.class into the multi release part
Of course it will be eventually put into meta-inf/versions/9  ... but my goal was to release with Java 8, so I need to save the .class file when compiling with Java 9+ before.

Andi


On 07.08.20 23:07, Uwe Schindler wrote:

> CleanerUtil dies not need to be multireleased. Just leave as is.
>
> The problem was only the missing import of the unsupported module which is required for unmapping. If you are in module mode at runtime (module path) and the unsupported module is not there, the exception handler falls through to the Java 8 code. After that it complains that nothing works.
>
> I still wonder where the exception came from. The code catches all of them and converts to a message which should get logged. So this is why I asked how you got the exception!
>
> I would recommend to put the module-info.class into the multi release part of the jar (below meta-inf/versions/9).
>
> Uwe
>
> Am August 7, 2020 8:18:48 PM UTC schrieb Andreas Beeker <[hidden email]>:
>> Thank you Uwe.
>>
>> The "jdk.unsupported" module hint did the trick - at least the build is
>> now getting over that error.
>>
>> So our build uses classpath mode when run under Java 8 and modulepath
>> for Java 9+ - actually everything except of the module-info is compiled
>> in classpath mode and the module-info is then patched into the classes.
>> To release under Java 8, I compile/update the module-info.class to the
>> source when compiling under Java 9+ - so this development noise is a
>> bit of a drawback which we might solve by using timestamp checks in the
>> build.
>>
>> I thought about providing a multi-release version of the CleanerUtil,
>> so we have only the Java 8 or the Java 9+ code active.
>> But then I'm puzzled how the JRE reacts, if you use classpath mode in
>> Java 9+.
>> I guess it ignores the module-info modules and the patched
>> multi-release class, effectively running the Java 8 code.
>> So we need both versions with that fallback mechanism.
>> TL;DR: don't change CleanerUtil.
>>
>>
>>
>>
>> On 07.08.20 18:24, Uwe Schindler wrote:
>>>> Sorry for not understanding the whole problem (module system usage
>> instaed
>>>> of classpath). Your code is perfectly valid and works with Java 9,
>> it's just broken
>>> ...with module system as its not fully declared all imports. The
>> compile won't find the issue, as it's in dynamic linking code.



signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Java 9+ modules / cleaner

Uwe Schindler
The warning printed by the code in Lucene (and the fork of Hadoop in POI) informs user. Btw, there's still "Hadoop" in your code. 😁

It's interesting that JDK prints the exception to stderr. This was not happening in Java 9. Looks like a bug.

Uwe

Am August 7, 2020 9:58:04 PM UTC schrieb Andreas Beeker <[hidden email]>:

>> The problem was only the missing import of the unsupported module
>which is required for unmapping.
>I think, I should also add the information about the permissions to a
>FAQ entry:
>https://stackoverflow.com/a/54046774/2066598
>
>> I still wonder where the exception came from.
>Have a look at AccessibleObject:349 which prints the stacktrace to
>System.Err
>
>> I would recommend to put the module-info.class into the multi release
>part
>Of course it will be eventually put into meta-inf/versions/9  ... but
>my goal was to release with Java 8, so I need to save the .class file
>when compiling with Java 9+ before.
>
>Andi
>
>
>On 07.08.20 23:07, Uwe Schindler wrote:
>> CleanerUtil dies not need to be multireleased. Just leave as is.
>>
>> The problem was only the missing import of the unsupported module
>which is required for unmapping. If you are in module mode at runtime
>(module path) and the unsupported module is not there, the exception
>handler falls through to the Java 8 code. After that it complains that
>nothing works.
>>
>> I still wonder where the exception came from. The code catches all of
>them and converts to a message which should get logged. So this is why
>I asked how you got the exception!
>>
>> I would recommend to put the module-info.class into the multi release
>part of the jar (below meta-inf/versions/9).
>>
>> Uwe
>>
>> Am August 7, 2020 8:18:48 PM UTC schrieb Andreas Beeker
><[hidden email]>:
>>> Thank you Uwe.
>>>
>>> The "jdk.unsupported" module hint did the trick - at least the build
>is
>>> now getting over that error.
>>>
>>> So our build uses classpath mode when run under Java 8 and
>modulepath
>>> for Java 9+ - actually everything except of the module-info is
>compiled
>>> in classpath mode and the module-info is then patched into the
>classes.
>>> To release under Java 8, I compile/update the module-info.class to
>the
>>> source when compiling under Java 9+ - so this development noise is a
>>> bit of a drawback which we might solve by using timestamp checks in
>the
>>> build.
>>>
>>> I thought about providing a multi-release version of the
>CleanerUtil,
>>> so we have only the Java 8 or the Java 9+ code active.
>>> But then I'm puzzled how the JRE reacts, if you use classpath mode
>in
>>> Java 9+.
>>> I guess it ignores the module-info modules and the patched
>>> multi-release class, effectively running the Java 8 code.
>>> So we need both versions with that fallback mechanism.
>>> TL;DR: don't change CleanerUtil.
>>>
>>>
>>>
>>>
>>> On 07.08.20 18:24, Uwe Schindler wrote:
>>>>> Sorry for not understanding the whole problem (module system usage
>>> instaed
>>>>> of classpath). Your code is perfectly valid and works with Java 9,
>>> it's just broken
>>>> ...with module system as its not fully declared all imports. The
>>> compile won't find the issue, as it's in dynamic linking code.

--
Uwe Schindler
Achterdiek 19, 28357 Bremen
https://www.thetaphi.de