Welcome! Please see the About page for a little more info on how this works.

0 votes
in Compiler by

Compiling a function that references a non loaded (or uninitialized) class triggers its init static. When the init static loads clojure code, some constants (source code I think) are leaked into the constants pool of the function under compilation.

It prevented CCW from working in some environments (Rational) because the static init of the resulting function was over 64K.

Steps to reproduce:

Load the leak.main ns and run the code in comments: the first function has 15 extra fields despite being identical to the second one.

`
(ns leak.main)

(defn first-to-load []
leak.Klass/foo)

(defn second-to-load []
leak.Klass/foo)

(comment
=> (map (comp count #(.getFields %) class) [first-to-load second-to-load])
(16 1)
)
`

`
package leak;

import clojure.lang.IFn;
import clojure.lang.RT;
import clojure.lang.Symbol;

public class Klass {
static {

RT.var("clojure.core", "require").invoke(Symbol.intern("leak.leaky"));

}
public static IFn foo = RT.var("leak.leaky", "foo");
}
`

`
(ns leak.leaky)

(defn foo
"Some doc"
[]
"hello")

(def unrelated 42)
`

https://gist.github.com/cgrand/5dcb6fe5b269aecc6a5b#file-main-clj-L10

Patch: clj-1620-v5.patch

33 Answers

0 votes
by

Comment made by: bronsa

Also Ghadi is right, infact replacing the whole body of thunk-fail.core with (:foo {}) is enough.

It would seem like the issue is with AOT (re)compiling top-level keyword lookup sites, my guess is that for some reason this patch is preventing correct generation of the __init static initializer.

0 votes
by

Comment made by: bronsa

I still have absolutely no idea what lein compile is doing but I figured out the issue.
The updated patch binds (in eval) the appropriate vars only when already bounded.

0 votes
by

Comment made by: alexmiller

Would it be worth using transients on the bindings map now?

0 votes
by

Comment made by: bronsa

Makes sense, updated the patch to use a transient map

0 votes
by

Comment made by: michaelblume

Is there a test we can add that'll fail in the presence of the v2 patch? preferably independent of the CLJ-1544 patch? I can try to write one myself, but I don't have a lot of familiarity with the Clojure compiler internals.

0 votes
by

Comment made by: bronsa

I'll have to think about a way to reproduce that bug, it's not a simple scenario to reproduce.
It involves compiling a namespace from an evaluated context.

0 votes
by

Comment made by: laurentpetit

Hello, is there any chance left that this issue will make it to 1.7 ?

0 votes
by

Comment made by: alexmiller

Wasn't planning on it - what's the impact for you?

0 votes
by

Comment made by: laurentpetit

The impact is that I need to use a patched version of Clojure for CCW.
While it's currently not that hard to follow clojure's main branch and regularly rebase on it or reapply the patch, it's still a waste of time.

0 votes
by

Comment made by: alexmiller

I will check with Rich whether it can be screened for 1.7 before we get to RC.

0 votes
by

Comment made by: alexmiller

same as v4 patch, but just has more diff context

0 votes
by

Comment made by: laurentpetit

the file mentioned in the patch field is not the right one IMHO

0 votes
by

Comment made by: alexmiller

which one is?

0 votes
by

Comment made by: laurentpetit

I think you previous comment relates to clj-1620-v5.patch, but at the end of the description there's the following line:

Patch: 0001-CLJ-1620-avoid-constants-leak-in-static-initalizer-v4.patch

0 votes
by

Comment made by: alexmiller

Those patches are equivalent with respect to the change they introduce; they just differ in how much diff context they have.

...