Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

Clojure resolves to wrong deftype classes when AOT compiling or reloading

Description

Compiling a class via `deftype` during AOT compilation gives different results for the different constructors. These hashes should be identical.

user=> (binding [*compile-files* true] (eval '(deftype Abc []))) user.Abc user=> (hash Abc) 16446700 user=> (hash (class (->Abc))) 31966239 ;; should be 16446700

This also means that whenever there's a stale AOT compiled deftype class in the classpath, that class will be used rather then the JIT compiled one, breaking repl interaction.

Another demonstration of this classloader issue (from CLJ-1495) when reloading deftypes (no AOT) :

user> (defrecord Foo [bar]) user.Foo user> (= (->Foo 42) #user.Foo{:bar 42}) ;;expect this to evaluate to true true user> (defrecord Foo [bar]) user.Foo user> (= (->Foo 42) #user.Foo{:bar 42}) ;;expect this to evaluate to true also -- but it doesn't! false user>

This bug also affects AOT compilation of multimethods that dispatch on a class, this affected core.match for years see http://dev.clojure.org/jira/browse/MATCH-86, http://dev.clojure.org/jira/browse/MATCH-98. David had to work-around this issue by using a bunch of protocols instead of multimethods.

Cause of the bug: currently clojure uses Class.forName to resolve a class from a class name, which ignores the class cache from DynamicClassLoader thus reloading deftypes or mixing AOT compilation at the repl with deftypes breaks, resolving to the wrong class.

Approach: the current patch (CLJ-979-v7.patch) addresses this issue in multiple ways:

  • it makes RT.classForName/classForNameNonLoading look in the class cache before delegating to Class/forName if the current classloader is not a DynamicClassLoader (this incidentally addresses also CLJ-1457)

  • it makes clojure use RT.classForName/classForNameNonLoading instead of Class/forName

  • it overrides Classloader/loadClass so that it's class cache aware – this method is used by the jvm to load classes

  • it changes gen-interface to always emit an in-memory interface along with the [optional] on-disk interface so that the in-memory class is always updated.

Patch: CLJ-979-v7.patch

Screened by: Alex Miller

Environment

None

Attachments

8
  • 16 Dec 2014, 06:50 PM
  • 15 Dec 2014, 11:43 PM
  • 12 Dec 2014, 07:07 PM
  • 12 Dec 2014, 04:09 PM
  • 12 Dec 2014, 03:38 PM
  • 12 Dec 2014, 12:06 PM
  • 11 Dec 2014, 07:40 PM
  • 29 Mar 2014, 08:27 PM

Activity

Show:

Nicola MomettoFebruary 18, 2015 at 10:21 AM

Colin, I opened CLJ-1663 with a patch that should fix the issue. Can try it and comment on that ticket if it does?

Colin FlemingFebruary 18, 2015 at 4:30 AM

I believe I have a case which is now broken because of this patch. I'm not 100% clear on the details so I might be wrong, but I can't see any other explanation for what I'm seeing. The bug I'm looking at is Cursive #748. Cursive calls into Leiningen in-process, and before doing that I create a new DynamicClassLoader which uses an IntelliJ PluginClassLoader as its parent. I believe that what is happening is that PluginClassLoader.loadClass() delegates to various parent classloaders then throws CNFE if it can't find the class in question. Am I correct in thinking that after this patch DCL now delegates to the parent classloader before checking its URL list, whereas previously it did not?

Michael BlumeJanuary 6, 2015 at 10:20 PM

New in-the-wild case: https://groups.google.com/forum/#!topic/clojure/WH6lJwH1vMg

Nicola MomettoDecember 16, 2014 at 6:50 PM

Updated patch with better tests, addressing Alex Miller's comments.

Alex MillerDecember 16, 2014 at 2:33 PM

Thanks Nicola. I'll certainly take over sheparding the bug and appeal to the greater community for help in broad testing when I think we're ready for that.

Completed

Details

Assignee

Reporter

Labels

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created May 3, 2012 at 9:01 AM
Updated February 18, 2015 at 10:21 AM
Resolved February 18, 2015 at 10:21 AM