deftype class literals and instances loaded from different classloaders when recompiling namespace

Description

Scenario: Given two files:

src/dispatch/core.clj:

src/dispatch/dispatch.clj:

Compile first core, then dispatch:

This scenario more commonly occurs in a leiningen project with :aot :all. Files are compiled in alphabetical order with :all. In this case, dispatch.core will be compiled first, then dispatch.dispatch.

Cause:

(compile 'dispatch.core)

  • transitively compiles dispatch.dispatch

  • writes .class files to compile-path (which is on the classpath)

  • assertion passes

(compile 'dispatch.dispatch)

  • due to prior compile, load dispatch.dispatch__init is loaded via the appclassloader

  • ->T constructor will use new bytecode to instantiate a T instance - this uses appclassloader, loaded from compiled T on disk

  • however, T class literals are resolved with RT.classForName, which checks the dynamic classloader cache, so uses old runtime version of T, instead of on-disk version

In 1.6, RT.classForName() did not check dynamic classloader cache, so loaded T from disk as with instances. This was changed in CLJ-979 to support other redefinition and AOT mixing usages.

Approaches:

1) Compile in reverse dependency order to avoid compiling twice.

Either swap the order of compilation in the first example or specify the order in project.clj:

This is a short-term workaround.

2) Move the deftype into a separate namespace from where it is used so it is not redefined on the second compile. This is another short-term workaround.

3) Do not put compile-path on the classpath (this violates current expectations, but avoids loading dispatch__init)

This is not easy to set up via Leiningen currently.

4) Compile each file with an independent Clojure runtime - avoids using cached classes in DCL for class literals.

Probably too annoying to actually do right now in Leiningen or otherwise.

5) Make compilation non-transitive. This is in the ballpark of CLJ-322, which is another can of worms. Also possibly where we should be headed though.

Screening: I do not believe the proposed patch is a good idea - it papers over the symptom without addressing the root cause. I think we need to re-evaluate how compilation works with regard to compile-path (#3) and transitivity (CLJ-322) (#5), but I think we should do this after 1.7. - Alex

See also: CLJ-1650

Environment

None

Activity

Show:
import
May 31, 2015, 2:55 AM

Comment made by: sfnelson

I've added a debug flag to my example that causes type instance hashcodes and their class-loaders to be printed.

Nicola Mometto
June 1, 2015, 1:23 PM

The compiler has weird loading rules when using `compile` and both a clj file and a class file are present in the classpath.

This bug happens because RT.load will load the AOT class file rebinding the ->Ctor to use the AOT deftype instance.

A fix for this would be making load "loaded libs" aware to avoid unnecessary/harmful reloadings.

Nicola Mometto
June 1, 2015, 4:55 PM

The attached patch fixes this bug by keeping track of what has already been loaded and loading the AOT class only if necessary

Alex Miller
June 16, 2015, 8:24 PM

Original description (since replaced):

Type-dispatching multimethods are defined using the wrong type instance

When using a multimethod that dispatches on types, such as print-dup/print-method, the type reference passed to addMethod in the presence of aot is incorrect on the second load of the namespace. This means that if the namespace has already been loaded as a dependency of another file, the second load when the namespace is loaded for aot compilation will produce a multimethod that fails to dispatch correctly.

I've created an example repository:
https://github.com/sfnelson/clj-mm-dispatch

To reproduce independently, create a namespace that contains a deftype and a multimethod dispatching on the type, and a second namespace that requires the first and sorts alphabetically before the first. Aot-compile both namespaces. When the type-defining namespace is loaded via require it produces a class file for the deftype. When it is loaded the second time for aot-compilation, the type corresponding to the existing class file is given to the defmethod, instead of the new class constructed by loading the namespace. This causes the multimethod it fail to dispatch correctly.

To me this issue seems similar to CLJ-979: the type passed to the multimethod is retrieved using the wrong classloader. This suggests that it might have wider implications than AOT and multimethod dispatch.

Nicola Mometto
June 18, 2015, 5:09 PM

I just realized this ticket is a duplicate of CLJ-1650

Assignee

Unassigned

Reporter

import

Approval

Vetted

Patch

Code

Affects versions

Priority

Major
Configure