jMonkeyEngine / jmonkeyengine

A complete 3-D game development suite written in Java.
http://jmonkeyengine.org
BSD 3-Clause "New" or "Revised" License
3.78k stars 1.12k forks source link

NativeLibraryLoader should not extract to working directory #1971

Closed Ali-RS closed 1 year ago

Ali-RS commented 1 year ago

Extracting natives from the classpath into the working directory is a risky hack. In the long term, JME should probably use a native-library loader analogous with that of LWJGL v3.

JME first looks working directory if it is writable then chooses that else it looks for user cache directory inside user.home directory, if found, will extract there, if not found then it will extract to userHome/.jme3.

Originally posted by @Ali-RS in https://github.com/jMonkeyEngine/jmonkeyengine/issues/1965#issuecomment-1444258551

We should omit the check for "working directory" step above.

pspeed42 commented 1 year ago

What happens if a user routinely runs applications that use different versions of JME? (like me)

Ali-RS commented 1 year ago

The application should still work as before. It will just extract and load natives from a different place.

pspeed42 commented 1 year ago

It seemed like you would be extracting all of the natives to the same place instead of the application-specific location.

...so if I were running my mapper tool in one window and tried to run jmec in another window then one of those applications might be corrupting the native libraries of another.

Ali-RS commented 1 year ago

Ah, now I see what you mean.

Probably that is why NativeLibraryLoader generates a native hash for naming the directory when extracting to UserCache... not sure thought.

https://github.com/jMonkeyEngine/jmonkeyengine/blob/375015f14c5f3cbda18302b3aa990e5f023e9da1/jme3-desktop/src/main/java/com/jme3/system/NativeLibraryLoader.java#L329

Ali-RS commented 1 year ago
 * to prevent collisions, a special subfolder is used
 * called natives_<hash> where <hash>
 * is computed automatically as the XOR of the classpath hash code
 * and the last modified date of this class.

See here

https://github.com/jMonkeyEngine/jmonkeyengine/blob/375015f14c5f3cbda18302b3aa990e5f023e9da1/jme3-desktop/src/main/java/com/jme3/system/NativeLibraryLoader.java#L340-L371

Does this settle the concern? or do you suggest a different solution?

Ali-RS commented 1 year ago

By the way, it is also possible to specify a custom location

https://github.com/jMonkeyEngine/jmonkeyengine/blob/375015f14c5f3cbda18302b3aa990e5f023e9da1/jme3-desktop/src/main/java/com/jme3/system/NativeLibraryLoader.java#L224-L235

Ali-RS commented 1 year ago

I also looked at LWJGL3 and from what I understand, when loading, it locks the file using java FileChannel and releases it after loading. So if another app tries to load it at the same time it will wait for the lock to release before loading.

See

https://github.com/LWJGL/lwjgl3/blob/b44ba7af8030200bee34d7dfb77edf5617589328/modules/lwjgl/core/src/main/java/org/lwjgl/system/SharedLibraryLoader.java#L232-L256

and before loading, it checks the file CRC hash with the one in the jar file, and if they do not match, it extract the native file again overwriting the old one. In JME we check this by comparing the last modified date of files.

So if another app corrupts the native file such that its CRC hash (in JME the last modified date) does not match it will always be replaced by a freshly extracted one.

This means we may not need to do the below step if we make use of file locks.


 * to prevent collisions, a special subfolder is used
 * called natives_<hash> where <hash>
 * is computed automatically as the XOR of the classpath hash code
 * and the last modified date of this class.

Please let me know what do you think

Ali-RS commented 1 year ago

Fixed in https://github.com/jMonkeyEngine/jmonkeyengine/pull/1973/commits/308cdf12fb1042d07866ab726b4835c492bd8d59

Instead of the working directory it extracts to the system temp directory retrieved by System.getProperty("java.io.tmpdir") if no custom extraction folder is specified.

The directory structure is like below

<java.io.tmpdir>/jme3/natives-<hash>  // hash is computed automatically for each app to prevent collisions