Clojure

Jar within a jar throws a runtime error

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.2, Release 1.3
  • Fix Version/s: Release 1.7
  • Component/s: None
  • Labels:
    None
  • Environment:
    Maven using the one-jar plugin
  • Patch:
    Code
  • Approval:
    Incomplete

Description

The code in RT.load() will load a class by name in several situations. One of these is when a .class resource exists, a .clj resource exists and the class is newer than the clj file. To determine the age of the class and clj, a call is made into RT.lastModified with the resource URL. If the URL is a jar protocol, then RT.lastModified() will try to open a connection to that jar, find the entry in the jar, and return its time.

Sometimes deployment occurs using nested jar files (for example: http://one-jar.sourceforge.net/) and custom classloaders. In this usage, the jar file can be opened, but the entry will not be found and an NPE will result during load:

Caused by: java.lang.NullPointerException
    at clojure.lang.RT.lastModified(RT.java:374)
    at clojure.lang.RT.load(RT.java:408)
    at clojure.lang.RT.load(RT.java:398)
    at clojure.lang.RT.doInit(RT.java:434)
    at clojure.lang.RT.<clinit>(RT.java:316)
... 7 more

Proposal: Make lastModified() more tolerant of finding a null entry and falling back to return the last modified date of the jar file itself.

Patch: clj-971-2.patch

Screened by:

  1. clj-971-1.patch
    24/Nov/13 7:11 AM
    1 kB
    Paavo Parkkinen
  2. clj-971-2.patch
    25/Nov/13 6:19 AM
    2 kB
    Paavo Parkkinen

Activity

Hide
Alex Miller added a comment -

Onejar creates a new kind of classpath container (which happens to have the same type as a .jar) but works differently. As such, resource urls that start with the jar protocol no longer work in certain ways (in this case returning null for lastModified). Java provides a mechanism to customize this behavior with a new url protocol, which is what the One-Jar-URL-Factory installs and uses. It thus seems to me that onejar introduces the issue and also provides the solution. As far as I see it, this is not a bug in Clojure itself.

If there were situations (without custom jar types or protocols) where lastModified() really does throw an NPE, then I would be more convinced that this was a scenario where Clojure should change behavior.

To modify the greeter example given in CLJ-1405:
1) bump the onejar-maven-plugin version to at least 1.4.5
2) add the following to the executions/execution/configuration for the plugin:

<manifestEntries>
      <One-Jar-URL-Factory>com.simontuffs.onejar.JarClassLoader$OneJarURLFactory</One-Jar-URL-Factory>
    </manifestEntries>
Show
Alex Miller added a comment - Onejar creates a new kind of classpath container (which happens to have the same type as a .jar) but works differently. As such, resource urls that start with the jar protocol no longer work in certain ways (in this case returning null for lastModified). Java provides a mechanism to customize this behavior with a new url protocol, which is what the One-Jar-URL-Factory installs and uses. It thus seems to me that onejar introduces the issue and also provides the solution. As far as I see it, this is not a bug in Clojure itself. If there were situations (without custom jar types or protocols) where lastModified() really does throw an NPE, then I would be more convinced that this was a scenario where Clojure should change behavior. To modify the greeter example given in CLJ-1405: 1) bump the onejar-maven-plugin version to at least 1.4.5 2) add the following to the executions/execution/configuration for the plugin:
<manifestEntries>
      <One-Jar-URL-Factory>com.simontuffs.onejar.JarClassLoader$OneJarURLFactory</One-Jar-URL-Factory>
    </manifestEntries>
Hide
Sean Shubin added a comment -

Alex,

Regarding scanning the jar, I was worried about the behavior not matching the intent of the code. As I have not had time to get familiar with the code, I figured this route was safer. If scanning the jar turns out to be overkill, so be it, we could just have the lastModified return 0 or something. Either way the code should be under test coverage, I will try to block out some time for that next weekend. Also, as I mentioned in CLJ-1405 (sorry about the duplicate issue), I did document the steps to reproduce the issue here: https://github.com/SeanShubin/clojure-one-jar.

Regarding Gleb's solution, that seems like a decent workaround, but I figured it would be better to fix the bug where it is rather than rely on a workaround. Shouldn't Clojure be changed if it is legitimately a bug in Clojure?

Show
Sean Shubin added a comment - Alex, Regarding scanning the jar, I was worried about the behavior not matching the intent of the code. As I have not had time to get familiar with the code, I figured this route was safer. If scanning the jar turns out to be overkill, so be it, we could just have the lastModified return 0 or something. Either way the code should be under test coverage, I will try to block out some time for that next weekend. Also, as I mentioned in CLJ-1405 (sorry about the duplicate issue), I did document the steps to reproduce the issue here: https://github.com/SeanShubin/clojure-one-jar. Regarding Gleb's solution, that seems like a decent workaround, but I figured it would be better to fix the bug where it is rather than rely on a workaround. Shouldn't Clojure be changed if it is legitimately a bug in Clojure?
Hide
Alex Miller added a comment -

Why is the solution that Gleb proposed not sufficient without changing Clojure? It seems fairly trivial to add a manifest line in the one-jar packaging step:

One-Jar-URL-Factory: com.simontuffs.onejar.JarClassLoader$OneJarURLFactory

Stu - I doubt the effort of setup is worth a test in the clojure tests but it might be useful to have documented steps to repro manually.

Sean - I think the effort on CLJ-1405 of scanning the full jar is going too far and not needed if we decide to go forward with this.

Show
Alex Miller added a comment - Why is the solution that Gleb proposed not sufficient without changing Clojure? It seems fairly trivial to add a manifest line in the one-jar packaging step: One-Jar-URL-Factory: com.simontuffs.onejar.JarClassLoader$OneJarURLFactory Stu - I doubt the effort of setup is worth a test in the clojure tests but it might be useful to have documented steps to repro manually. Sean - I think the effort on CLJ-1405 of scanning the full jar is going too far and not needed if we decide to go forward with this.
Hide
Sean Shubin added a comment - - edited

I notice this solution falls back on the last modified date for the entire jar, while it is still possible to get the date of the individual file, albeit less efficiently.
I posted a way to get the individual file date in CLJ-1405.
Perhaps you don't need the date of the individual file, but I am having a hard time groking the calling code so I am not sure what its intent is.
I did notice in the calling code that the if statement
(classURL != null && (cljURL == null || lastModified(classURL, classfile) > lastModified(cljURL, cljfile))) || classURL == null
Is logically equivalent to the much simpler
classURL == null || cljURL == null || lastModified(classURL, classfile) > lastModified(cljURL, cljfile)

Show
Sean Shubin added a comment - - edited I notice this solution falls back on the last modified date for the entire jar, while it is still possible to get the date of the individual file, albeit less efficiently. I posted a way to get the individual file date in CLJ-1405. Perhaps you don't need the date of the individual file, but I am having a hard time groking the calling code so I am not sure what its intent is. I did notice in the calling code that the if statement (classURL != null && (cljURL == null || lastModified(classURL, classfile) > lastModified(cljURL, cljfile))) || classURL == null Is logically equivalent to the much simpler classURL == null || cljURL == null || lastModified(classURL, classfile) > lastModified(cljURL, cljfile)
Hide
Stuart Halloway added a comment -

Can we get a test showing the normal path and the can't-read-inside path?

Show
Stuart Halloway added a comment - Can we get a test showing the normal path and the can't-read-inside path?
Hide
Gleb Kanterov added a comment -

I had the same issue, adding following line to manifest file worked for me

One-Jar-URL-Factory: com.simontuffs.onejar.JarClassLoader$OneJarURLFactory
Show
Gleb Kanterov added a comment - I had the same issue, adding following line to manifest file worked for me
One-Jar-URL-Factory: com.simontuffs.onejar.JarClassLoader$OneJarURLFactory
Hide
Paavo Parkkinen added a comment -

New patch with duplicate calls to url.openConnection() factored out.

Show
Paavo Parkkinen added a comment - New patch with duplicate calls to url.openConnection() factored out.
Hide
Alex Miller added a comment -

Thanks Paavo - on a quick scan, it looks like in the case you're interested in the updated code would now call url.openConnection() twice - perhaps that could be factored out?

Show
Alex Miller added a comment - Thanks Paavo - on a quick scan, it looks like in the case you're interested in the updated code would now call url.openConnection() twice - perhaps that could be factored out?
Hide
Paavo Parkkinen added a comment -

Took the code change in the description and rolled it into a patch to hopefully push this forward a little bit.

Show
Paavo Parkkinen added a comment - Took the code change in the description and rolled it into a patch to hopefully push this forward a little bit.
Hide
Anders Sveen added a comment -

Would be awesome if you could get this working. Wanting to use som Clojure libs in Java so Leiningen uberjar is not an option right now.

Show
Anders Sveen added a comment - Would be awesome if you could get this working. Wanting to use som Clojure libs in Java so Leiningen uberjar is not an option right now.

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated: