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.

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

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

Activity

Show:
Alex Miller
December 16, 2014, 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.

Nicola Mometto
December 16, 2014, 6:50 PM

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

Michael Blume
January 6, 2015, 10:20 PM

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

Colin Fleming
February 18, 2015, 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?

Nicola Mometto
February 18, 2015, 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?

Completed

Assignee

Unassigned

Reporter

import

Approval

Ok

Patch

Code and Test

Fix versions

Priority

Critical
Configure