so, it is important in this case because of how the body of the try gets lifted into a function, but it should not matter in 99.999% of normal cases.
what was happening before is that the locking macro expanded to a try not in the tail position, and that lifts the try body into a function (around which you can do catch/finally kinds of stuff), which means the monitorenter was in the original body and the monitorexit was in the lifted function (with the lockee as a closed over field in the function object). because these are in different methods and in a field, which gets loaded and cleared due to locals clearing, the graal analyzers can't connect that the monitorexit is attached to the monitorenter and it looks unbalanced.
the outer let in the new locking macro evaluates the lock object, that goes into the the next try block. the inner let puts it in a local, which will be not cleared due to the outer try (as it might be needed if there is a finally), the lock object then ends up on the stack for both enter and exit.
so the outer try here influences locals clearing just to make the lockee tracking easier for the graal analyzer.
needless to say, this is super subtle
I think this could easily also be a do
- not sure if there is any good reason to use let [] there.
could just be historical
Here’s what the ‘locking’ problem (CLJ-1472) was with Graal and why the patch fixes it: The locking macro emits a try block. When not in tail position, try blocks get lifted into lambdas, and closed over variables (including, importantly, the lockee) become ‘fields’ of the lambda object. Thus in that situation the monitor enter/exit calls were being made on a field. Graal doesn’t trust that the value of that field will be the same in the enter and exit calls, so it balks that they might not be a balanced pair. The solution is to get the lockee onto the stack so Graal can understand that it is the same target. The problem has nothing to do with exception handling blocks or failure to emulate javac’s infinite loop trick. How the patch works: We need to get the lockee on the stack, even when lifting occurs, and use the same stack variable in try/finally. Thus we need a let around try/finally. But lifting can still happen, so that let needs to be in what’s lifted. That’s the role of the outer try. The outer try will be lifted, and the let with it, the let will put the field on the stack, Graal sees enter/exit on the same stack value and its verifier is thus happy. The inner try will always be in tail position.
Thanks Rich for the detailed explanation!
yeah, like I said! ;)
@lee Might be good to document this in clj-graal-docs 🙂
agreed, will do later today!
Updated our clj-graal-docs README (and scripts) to reflect my understanding of current state of CLJ-1472 https://github.com/lread/clj-graal-docs/blob/master/CLJ-1472/README.md
patches were just applied today in clojure and spec.alpha
cool!
spec.alpha 0.2.187 is available (that alone gets you past the spec issue, but not the rest)
clojure 1.10.2-master-SNAPSHOT should be available at some point, we haven't done an actual release there yet - but that includes CLJ-1472 and CLJ-2502, and depends on spec.alpha 0.2.187
the plan is to release this stuff relatively soon, maybe with a few additional fixes
that's very good news, thank you
I am still on the fence about how to think about the reflector thing
I can ask about this on the graalvm github issue tracker
I expect clojure to be in this situation for a while (and then it will be replaced by some other java-version-dependent thing), so your approach of just overriding it is not an option in clojure itself
there are a variety of approaches to jvm-specific versions (classifiers, jars that support multiple runtimes, etc) but those are all pretty big changes
I don't know, still thinking about it
I understand. Luckily the workaround can be applied locally to final projects.
btw, here's a trivial typo patch, might you be interested in getting in something low risk: https://clojure.atlassian.net/browse/CLJ-2444
more looking for high priority than trying to shove a bunch of stuff in
I logged an issue about clojure.lang.Reflector + java11 here: https://github.com/oracle/graal/issues/2214
Hmm, adding --report-unsupported-elements-at-runtime
does make this repro work:
$ ./reflector-test
11.0.6
GraalVM 20.0.0 CE
Can access method: true
so maybe it's already solved in their v20. More testing: this also works with 19.3.0 java11. So maybe that's the way to deal with this after all.Confirmed that the new version of spec solves the 'locking' issue when the graalvm analyzer hits spec's dynaload:
{:deps {org.clojure/clojure {:mvn/version "1.10.1"}
org.clojure/spec.alpha {:mvn/version "0.2.187"}}}
$ ./spec-test
{:major 1, :minor 10, :incremental 1, :qualifier nil}
true
🎉Thanks for dedicating time to this Alex and Rich, this makes me very happy 🙂.
Are there known solutions to the <http://java.io|java.io>.FilePermission
issue described at https://github.com/BrunoBonacci/graalvm-clojure/blob/master/aleph/README.md#caveats, which comes up with 20.0.0-java11? Running into it with another library.
I guess use 19.3.1 java11 or 20.0.0 java 8?
I see an issue about it here: https://github.com/oracle/graal/issues/2177 Not sure if that's the same problem
If it is, it's maybe good to communicate you're experiencing the same problem and give some details if you have anything to add there
(using different versions definitely solves the issue, just trying to figure out what's even causing it)
seems like it's higher up the food chain than clojure, though
might be a bug. there's also a native image / graalvm slack: https://app.slack.com/client/TN37RDLPK making a Java-only repro is certainly preferred if possible if you're going to submit an issue, but they also accept uberjars or detailed build instructions
great, thanks!
for posterity -- it was an issue of accessing resources and needing to assist the native-image builder with links to the resource files, which means sticking them in META-INF
and utilizing (https://github.com/kumarshantanu/lein-javac-resources) when running lein-native-image
@wcohen the -H:IncludeResources=...
option?
uberjarring the class to be native-imaged, running the uberjar with native-image-agent=config-output-dir
per https://github.com/oracle/graal/blob/master/substratevm/CONFIGURE.md, then adding content from those jsons to META-INF/native-image
@wcohen There is an issue here https://github.com/lread/clj-graal-docs/issues/13 to write some docs about it. More than welcome if you feel like contributing.
will do in the next few days!