ShapefileReadWriteTest fail on Windows

Description

In the gt-shapefile module the ShapefileReadWriteTest unit tests fail on Windows. Something to do with memory-mapped files.

Environment

Windows 10, Java 11

Activity

Show:
Christian Sommer
March 8, 2019, 6:08 AM
Edited

I discovered the same issue. This is also true for GeoTools versions 21.0 and 21-RC1 (Tested Oracle JDK, OpenJDK, AdoptOpenJDK 11.0.2)

Our application reads a shapefile and wants to write it afterwards, but this is not possible since the MappedByteBuffer was not yet GCed and NIOUtilities.clean() is no longer forcing the close of the buffer. So shapefiles are currently not useable for us under JDK 11.

I did some research & debugging on this topic. NIOUtilities.clean() has a comment that Java 9+ fixed this behaviour and no force-close is required, but this is IMHO not true. I could not find anything in the openjdk bug system and they even introduced a new sun.misc.Unsafe method for this special case, which does not throw the "illegal reflection" warning in JDK 9+. See https://bugs.openjdk.java.net/browse/JDK-8171377

So, I think the NIOUtilities.clean() method should always invoke the cleaner methods through reflection, for Java < 9 the old method, new invokeCleaner otherwise (And it would be probably a good idea to run all tests against a JDK 8 and 11 in the future).

I did a small hack for this issue, which fixed the issue for us (attention - Java 9+ only):

Andrea Aime
March 8, 2019, 3:10 PM
Edited

Bizarre, we have Appveyor Windows build server running on the code at each PR and the issue hasn't shown up.... aaah, but it's a JDK 8 only build (Appveyor is giving us limited parallel builds, and we are already using a lot on Travis too... we'd need another provider, if you know one and are interested in setting it up, I'm all ears).

That said, memory mapping is indeed not recommended for Windows (the parameter also says so if memory serves me right).
the approach is interesting but we need something that would run on JDK 8 too

Leif Gruenwoldt
March 13, 2019, 2:01 AM

I re-ran the tests with Java 8 and all tests pass there. So it's certainly sounding like a Java 11 issue on Windows.

Christian Sommer
March 13, 2019, 7:35 AM

Concercing Build Servers: I don't know of any better fitting provider, sorry

Concering the buffer cleanup bug:

The current solution is correct for Java <= 8, so we don't need to change this. There simply needs to be some switch in NIOUtilities.clean(buffer) for java >=9 which invokes the other cleaner method I have posted above.

I could provide a PR on your github repo if you like. The change should be really small and only requires changes on NIOUtilities.java clean+getCleaner methods.

Andrea Aime
March 16, 2019, 10:05 AM

Assignee

Unassigned

Reporter

Leif Gruenwoldt

Triage

None

Components

Affects versions

Priority

Low
Configure