The close is safe as long as the key entry is removed at the same time - and that the removal is synchronized on the factory. I have seen several implementations that close the jar but then leave the closed jar in the map, and that is very 'game over'. I have looked at the Java source to try and ensure that what I am doing is safe, and have tested a cycling redeployment over several hours.
Based on some of the original code comments and existing code I believe that all I have done is to 'fix' and enhance the originally intended functionality that was broken. In particular in ClassLoaderUtil.clearSunJarFileFactoryCacheImpl: JarFile jarFile = (JarFile) fileCache.remove(url); The old line can return null because the key is not always a URL (At least since JDK 1.6.0), it is now a String that starts with 'file:///', but I think there was a period where either a URL or a String was being used as a key (pre generics), and this is what led to intermittent behaviour. On Windows URI or URL will return (toString, toExternalForm etc.) 'file://C:/blah.jar' and it needs to be 'file:///C:/blah.jar' (note the extra slash). The JarFileFactory fudges the key together internally, and I have changed the code to do the same. The first fall back option is to use the toString. To that end, the JarFile was not always being removed and closed, and was 'sometimes' locked on Windows systems, dependent on the JDK version. On Unix like systems it has nearly always been possible to delete or replace these 'used' jar files without issue because a file lock is not obtained until a physical read operation occurs. The 'temp file' scenario (Revision: 637617) seems to have only been developed due to these unresolved file locking issues in the past (most likely caused by this very issue), and I feel that after my tests is actually no longer required. I left it in because I did not want to make too many changes to this very 'touchy' area, and it is also very safe to do it this way. My previous modification in this class was to address the ConcurrentModificationException, which was simply caused by an unsynchronized iteration over the map. What I had not noticed was that the JarFile was null, but I hadn't actually been looking for it until now. Good that you forced me to rethink over this, as I have also now just added the pre JDK 1.6 fallback, which is to still use the URL... Doh! Also good to know that JDK 7 has actually got a ClassLoader.close method, let's just hope that they get it right! Andy. -- View this message in context: http://openejb.979440.n4.nabble.com/ClassLoader-File-Locking-Issues-Fixed-tp3071131p3077795.html Sent from the OpenEJB Dev mailing list archive at Nabble.com.
