jblas-project / jblas

Linear Algebra for Java
http://jblas.org
BSD 3-Clause "New" or "Revised" License
590 stars 149 forks source link

temp files existance check fix #62

Closed boscogh closed 9 years ago

boscogh commented 9 years ago

this is the same as pull request #61 fixes issue #59 see my last comment there https://github.com/mikiobraun/jblas/pull/61

boscogh commented 9 years ago

this is very simple test case - but it is for windows only: it shows that you could't delete the file under loaded dll. but if you unload dll, you would be able to delete the file I used "Files.delete" because it more clearly show that the error is.

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Vector;
public class DllLoadUnload {
public static void main(String[] args) throws Throwable {
    String dllName = "C:\\Temp\\test1\\libgcc_s_dw2-1.dll";
    System.load(dllName);
    //Try to comment this out - and delete will fail
    unloadNativeLibs();
    Files.delete(Paths.get(dllName, new String[] {}));
}

public static void unloadNativeLibs() throws Throwable {
    ClassLoader classLoader = DllLoadUnload.class.getClassLoader();
    Field field = ClassLoader.class.getDeclaredField("nativeLibraries");
    field.setAccessible(true);
    Vector libs = (Vector) field.get(classLoader);
    for (Object o : libs) {
        Method finalize = o.getClass().getDeclaredMethod("finalize", new Class[0]);
        finalize.setAccessible(true);
        finalize.invoke(o, new Object[0]);
    }
}
}
mikiobraun commented 9 years ago

Hi boscogh,

ok, I've pondered this question very hard and came up with a solution which

  1. goes trough old temp files on startup and removes old files it finds.
  2. because it cannot know whether these files are still used, it just tries to delete them and then Windows "can't delete if still open" makes sure we don't accidentally delete old files.
  3. In order to make sure we don't accidentally delete old files, I added a interleaved FileInputStream before I close the writing stream and before I load the library to make sure it doesn't accidentally get deleted before the library is loaded.

IMHO this is a cleaner solution than the one you suggested. My biggest concern is that another jblas instance might see that the file exists and try to load it before it is fully copied - and fail, leading to hard to debug and detect startup issues.

In any case, thanks for the discussion! That helped a lot to improve this aspect of jblas on the Windows!

I'll add a note to the RELEASE_NOTES -M