The code is far from optimal and does require a clean up and a second look
for sure. I am on it already. The revert was back to something that was
known to be working until a better solution is implemented. The changes you
made broke redeployment on my dev box - Files were back to being locked, and
the ConcurrentModifiedException occurring.
The original issue stemmed from the fact that unloading began to not release
jar files in 'some' circumstances. In fact at one point 'no' jar files were
released by the VM and that led to a whole lot of 'anti jar locking' code
which is imho not required if we get this code really optimised. The problem
was introduced by the VM.
Sun changed sun.net.www.protocol.jar.JarFileFactory several times and access
was not consistent - I think their guys just kept toying with it which led
to the key confusion.
The ConcurrentModifiedException stems from the fact that removing a single
jar may also trigger the removal of other related jars internally. As we can
only do this via reflection there is no 'real' way to synchronize
internally, which is why I added the retry recursion. As a quick fix I now
copy the keys in the last commit, which should at least prevent a
ConcurrentModifiedException (I was sure I had that already?) on the OpenEJB
side.
There are two maps 'fileCache' (the url to file cache) and 'urlCache' (the
file to url cache) which basically do the same (hold a reference to the
physical JarFile file, but in reverse). Another issue is that JarFile
implementation signatures have changed several times, so using it as a key
has not been reliable.
Using Map<JarFile, URL> and Map<URL, JarFile> is not going to
work on all 1.6 runtimes as these maps have used different keys internally.
The urlCache key is 'now' the current JarFile. The fileCache key is now a
URL. I'll talk about past keys in a mo.
Both fileCache.put() and urlCache.put() have not been consistent, meaning
that there may be mixed key types even within the same runtime (Generics
would have fixed this, but the current runtime still uses an untyped
HashMap!).
This actual file locking issue is purely a Win OS issue. Under Linux/mac
there is no file locking on loaded jars. They can be deleted and the VM will
throw out an internal stacktrace - Putting them back is just fooling the VM
into reloading. I am sure this is bad, but so far everyone seems to get away
with it.
In the for (final Object item : keys) iteration you are right about not
closing the JarFile, as the code should just be looking for and building
valid URLs - Not sure how the close got in there! The URLs are only added to
the urls list if they match the criteria (null != file && null != url &&
isParent(jarLocation, file)). The scope for change here is that we are
basically blind as to what type is being returned across all runtime/os
versions, and need to try and cater for all reasonable options.
The second iteration for (final URL jar : urls) should do everything
possible to locate and remove the actual references in the maps and close
the JarFile.
So far I have seen the following used as mixed keys internally:
1. A broken file URI String starting with file:/// - Note the three forward
slashes.
2. JarFile.toExternalForm()
3. JarFile object
4. URL object
5. URL String
6. URI object.
7. URI String
8. A File
So it is possible to get the URL object from the urlCache map but it may be
used as a String key in the other map?
The clean up just requires that the remove method is called on the fileCache
using the following keys.
JarFile
JarFile.toExternalForm()
JarFile.toString()
URL
URL.toString()
URL.toExternalForm()
URI
URI.toString()
URI.toASCIIString()
("file:///" + new
File(URI.create(jar.toString())).getAbsolutePath().replace('\\', '/'));
--
View this message in context:
http://openejb.979440.n4.nabble.com/Re-svn-commit-r1146747-openejb-trunk-openejb3-container-openejb-core-src-main-java-org-apache-openeja-tp3674308p3677829.html
Sent from the OpenEJB Dev mailing list archive at Nabble.com.
