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-2.patch
    25/Nov/13 6:19 AM
    2 kB
    Paavo Parkkinen
  2. clj-971-1.patch
    24/Nov/13 7:11 AM
    1 kB
    Paavo Parkkinen

Activity

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.
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
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 -

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
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
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
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
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 -

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 -

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>

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated: