Clojure

The locking macro fails bytecode verification on ART runtime

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.6
  • Fix Version/s: Release 1.11
  • Component/s: None
  • Labels:
  • Environment:
    Android ART runtime
  • Patch:
    Code
  • Approval:
    Screened

Description

Android ART runs compile time verification on bytecode and fails on any usage of the locking macro. The error looks like that seen in CLJ-1829 (in that case clojure.core.server/stop-server calls the locking macro):

10-16 14:49:26.801 2008-2008/? E/AndroidRuntime: java.lang.VerifyError: Rejecting class clojure.core.server$stop_server because it failed compile-time verification (declaration of 'clojure.core.server$stop_server' appears in /data/app/com.clojure_on_android-1/base.apk)

Cause: From discussion on an Android issue (https://code.google.com/p/android/issues/detail?id=80823), it seems this is due to more strictly enforcing the "structural locking" provisions in the JVM spec (https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-2.html#jvms-2.11.10).

It appears that the mixture of monitor-enter, monitor-exit, and the try/finally block in `locking` create paths that ART is flagging as not having balanced monitorenter/monitorexit bytecode. Particularly, monitorenter and monitorexit themselves can throw (on a null locking object). The Java bytecode does some tricky exception table handling to cover these cases which (afaict) is not possible to do without modifying the Clojure compiler.

Approach: One possible approach is to have locking invoke the passed body in the context of a synchronized block in Java. This avoids the issue of the tricky bytecode by handing it over to Java but has unknown performance impacts.

Patch: clj-1472-3.patch

See also: Examination of the bytecode as compared to a java synchronized block shows up a number of differences:
https://gist.github.com/AdamClements/2ae6c4919964b71eb470

Screened by: Alex Miller - I'm marking this screened as I think it is a viable approach that fixes the issue and due to the infrequency of its use, I'm not really that concerned about it being a performance problem. I will flag that I think another way to handle this would be to make `locking` a special form with compiler support, but I'm not sure whether that's worth doing or not, so I will leave that to Rich to decide.

  1. 0001-CLJ-1472-Locking-macro-without-explicit-monitor-ente.patch
    28/Nov/14 11:03 AM
    1 kB
    Adam Clements
  2. clj-1472.patch
    21/Dec/15 7:44 PM
    2 kB
    Alex Miller
  3. clj-1472-2.patch
    22/Dec/15 1:46 PM
    2 kB
    Alex Miller
  4. clj-1472-3.patch
    12/Dec/18 12:27 PM
    2 kB
    Alex Miller

Activity

Hide
Adam Clements added a comment -

After using this a little more, I've found that moving this outside the try block breaks nREPL.

Looking at the bytecode, the monitorenter for the locking in clojure.tools.nrepl.middleware.session/session-out and in a few other places ends up in an entirely different method definition and we now get a JVM IllegalMonitorStateException as well as an ART verification error for this function.

Show
Adam Clements added a comment - After using this a little more, I've found that moving this outside the try block breaks nREPL. Looking at the bytecode, the monitorenter for the locking in clojure.tools.nrepl.middleware.session/session-out and in a few other places ends up in an entirely different method definition and we now get a JVM IllegalMonitorStateException as well as an ART verification error for this function.
Hide
Andy Fingerhut added a comment -

Adam, I cannot comment on whether your patch is of interest or not, but it is true that no patch will be committed to Clojure if the author has not signed a Contributor Agreement, which can now be done on-line at http://clojure.org/contributing

Show
Andy Fingerhut added a comment - Adam, I cannot comment on whether your patch is of interest or not, but it is true that no patch will be committed to Clojure if the author has not signed a Contributor Agreement, which can now be done on-line at http://clojure.org/contributing
Hide
Adam Clements added a comment - - edited

Uploaded a new patch (and signed the contributor agreement). This passes both the JVM and ART bytecode verification, The extra try/catch around the monitor exit is optional (verification passes with or without it) but where the java version retries monitor-exit indefinitely and shows the deadlock at the right time, without catching errors in the monitor-exit an undetermined monitor-enter in the future might fail, not showing up the actual bug.

It's not very pretty, but without finer grained control of the generated bytecode, this is the best I could do.

Show
Adam Clements added a comment - - edited Uploaded a new patch (and signed the contributor agreement). This passes both the JVM and ART bytecode verification, The extra try/catch around the monitor exit is optional (verification passes with or without it) but where the java version retries monitor-exit indefinitely and shows the deadlock at the right time, without catching errors in the monitor-exit an undetermined monitor-enter in the future might fail, not showing up the actual bug. It's not very pretty, but without finer grained control of the generated bytecode, this is the best I could do.
Hide
Adam Clements added a comment -

Have just tested with Lollipop, and this patch might no longer be sufficient.

Getting in touch with the ART guys to see if they can shed a little more light and verify whether it will work on the current master branch of AOSP

Show
Adam Clements added a comment - Have just tested with Lollipop, and this patch might no longer be sufficient. Getting in touch with the ART guys to see if they can shed a little more light and verify whether it will work on the current master branch of AOSP
Hide
Adam Clements added a comment -

Bug filed with AOSP project, hopefully they can shed some light on whether it is our problem and if so how we can fix it.

https://code.google.com/p/android/issues/detail?id=80823

Show
Adam Clements added a comment - Bug filed with AOSP project, hopefully they can shed some light on whether it is our problem and if so how we can fix it. https://code.google.com/p/android/issues/detail?id=80823
Hide
Adam Clements added a comment - - edited

I have uploaded an alternative implementation of the locking macro (0001-CLJ-1472-Locking-macro-without-explicit-monitor-ente.patch) which cheats a little - the synchronized block is actually implemented in Java and so guarantees compatibility. This is at the cost of a little extra indirection and the naming/location could probably be better.

But it does fix the bug and work on all versions of android, android + art and the jvm. Would this approach be acceptable?

Show
Adam Clements added a comment - - edited I have uploaded an alternative implementation of the locking macro (0001-CLJ-1472-Locking-macro-without-explicit-monitor-ente.patch) which cheats a little - the synchronized block is actually implemented in Java and so guarantees compatibility. This is at the cost of a little extra indirection and the naming/location could probably be better. But it does fix the bug and work on all versions of android, android + art and the jvm. Would this approach be acceptable?
Hide
Kevin Downey added a comment -

I have yet to see any evidence that the bytecode clojure is generating in some way violates the jvm spec, so I suspect the issue is clojure requires a jvm to run, and android doesn't provide a jvm, just something that looks like one if you don't tread outside the beaten path.

Show
Kevin Downey added a comment - I have yet to see any evidence that the bytecode clojure is generating in some way violates the jvm spec, so I suspect the issue is clojure requires a jvm to run, and android doesn't provide a jvm, just something that looks like one if you don't tread outside the beaten path.
Hide
Kevin Downey added a comment -

given the structured locking verbiage in https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-2.html#jvms-2.11.10, `(locking nil)` may generate bytecode whose runtime behavior violates structured locking. the first patch on this issue can cause the compiler to emit monitorenter/exit instructions in different methods, which definitely violates structured locking

Show
Kevin Downey added a comment - given the structured locking verbiage in https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-2.html#jvms-2.11.10, `(locking nil)` may generate bytecode whose runtime behavior violates structured locking. the first patch on this issue can cause the compiler to emit monitorenter/exit instructions in different methods, which definitely violates structured locking
Hide
Adam Clements added a comment -

Yes, the first patch was definitely wrong, I left it for some context to the conversation, but it's probably best to just remove it for clarity.

For anyone following this conversation who doesn't want to decompile and observe the bytecode, here's a gist with the difference between a java synchronized block and clojure locking https://gist.github.com/AdamClements/2ae6c4919964b71eb470

I'm finding it hard to work out where the deviation from the spec occurs too, though I can see the difference with the Java version, if anything, the Clojure version looks closer to what's described in the spec than the Java one!

If someone with more knowledge than me on the subject could engage on the AOSP bug https://code.google.com/p/android/issues/detail?id=80823 then perhaps we could settle this as an android bug which is too focussed on the java implementation rather than the JVM spec, or perhaps they'll find something that's wrong with the Clojure implementation. I have uploaded the original clojure behaviour and asked them for some more explanation on why it is failing.

Show
Adam Clements added a comment - Yes, the first patch was definitely wrong, I left it for some context to the conversation, but it's probably best to just remove it for clarity. For anyone following this conversation who doesn't want to decompile and observe the bytecode, here's a gist with the difference between a java synchronized block and clojure locking https://gist.github.com/AdamClements/2ae6c4919964b71eb470 I'm finding it hard to work out where the deviation from the spec occurs too, though I can see the difference with the Java version, if anything, the Clojure version looks closer to what's described in the spec than the Java one! If someone with more knowledge than me on the subject could engage on the AOSP bug https://code.google.com/p/android/issues/detail?id=80823 then perhaps we could settle this as an android bug which is too focussed on the java implementation rather than the JVM spec, or perhaps they'll find something that's wrong with the Clojure implementation. I have uploaded the original clojure behaviour and asked them for some more explanation on why it is failing.
Hide
Adam Clements added a comment -

The response from the ART guys about what they think we're violating is:

The section on "Structured locking" contains the following:

"[...] implementations [...] are permitted but not required to enforce
both of the following two rules guaranteeing structured locking. [...]"

ART currently enforces both rules at verification time, including

"At no point during a method invocation may the number of monitor exits
performed by T on M since the method invocation exceed the number of
monitor entries performed by T on M since the method invocation."

Show
Adam Clements added a comment - The response from the ART guys about what they think we're violating is: The section on "Structured locking" contains the following: "[...] implementations [...] are permitted but not required to enforce both of the following two rules guaranteeing structured locking. [...]" ART currently enforces both rules at verification time, including "At no point during a method invocation may the number of monitor exits performed by T on M since the method invocation exceed the number of monitor entries performed by T on M since the method invocation."
Hide
Adam Clements added a comment -

If for example instruction https://gist.github.com/AdamClements/2ae6c4919964b71eb470#file-test_locks-class-L24 or the monitor-enter itself on the next line were to fail, couldn't it could end up in the finally clause and attempt to release the lock even though it has never been captured?

I think this violates the structured locking rules in the jvm spec you linked to.

Show
Adam Clements added a comment - If for example instruction https://gist.github.com/AdamClements/2ae6c4919964b71eb470#file-test_locks-class-L24 or the monitor-enter itself on the next line were to fail, couldn't it could end up in the finally clause and attempt to release the lock even though it has never been captured? I think this violates the structured locking rules in the jvm spec you linked to.
Hide
Kevin Downey added a comment -

an interesting question about structured locking, is does the specification refer to the static bytecode or the runtime behavior of the bytecode. given the bytecode linked(https://gist.github.com/AdamClements/2ae6c4919964b71eb470#file-test_locks-class-L24), the static bytecode has the same number of entries and exits, but the dynamic behavior may be different. I wonder which one the art guys claim to be enforcing at verification time (it seems like it would have to be the static bytecode, not the dynamic properties, but then they shouldn't be failing to verify this). looking at the google code issue, the comment https://code.google.com/p/android/issues/detail?id=80823#c6 was made by the same dev as https://code.google.com/p/android/issues/detail?id=80823#c3, so I sort of suspect there is some miscommunication going on. It is not clear in what context the dev is replying in, since in the previous comment you mention splitting monitor-enter and exit across methods. I think things would be much clearer if all patches, specialized clojure android builds, etc, were gotten rid of, then with a vanilla clojure jar you get a javap dump of what fails to verify, then just take that over to the android issue tracker and ask "hey, this fails to verify, why?"

Show
Kevin Downey added a comment - an interesting question about structured locking, is does the specification refer to the static bytecode or the runtime behavior of the bytecode. given the bytecode linked(https://gist.github.com/AdamClements/2ae6c4919964b71eb470#file-test_locks-class-L24), the static bytecode has the same number of entries and exits, but the dynamic behavior may be different. I wonder which one the art guys claim to be enforcing at verification time (it seems like it would have to be the static bytecode, not the dynamic properties, but then they shouldn't be failing to verify this). looking at the google code issue, the comment https://code.google.com/p/android/issues/detail?id=80823#c6 was made by the same dev as https://code.google.com/p/android/issues/detail?id=80823#c3, so I sort of suspect there is some miscommunication going on. It is not clear in what context the dev is replying in, since in the previous comment you mention splitting monitor-enter and exit across methods. I think things would be much clearer if all patches, specialized clojure android builds, etc, were gotten rid of, then with a vanilla clojure jar you get a javap dump of what fails to verify, then just take that over to the android issue tracker and ask "hey, this fails to verify, why?"
Hide
Adam Clements added a comment -

Yeah, I shouldn't have confused it with the patched versions. The gist and the currently uploaded version use the vanilla clojure version of the locking macro now though.

I think the issue comes from the exception table and the instructions that covers. If line 24 can throw for example, you would end up at runtime with a monitor-exit, having never encountered a monitor-enter.

Show
Adam Clements added a comment - Yeah, I shouldn't have confused it with the patched versions. The gist and the currently uploaded version use the vanilla clojure version of the locking macro now though. I think the issue comes from the exception table and the instructions that covers. If line 24 can throw for example, you would end up at runtime with a monitor-exit, having never encountered a monitor-enter.
Hide
Ghadi Shayban added a comment -

According to Marcus Lagergren JRockit and Hotspot both account for lock releases being in a different method... Perhaps Android has a different (wrong) interpretation?

Show
Ghadi Shayban added a comment - According to Marcus Lagergren JRockit and Hotspot both account for lock releases being in a different method... Perhaps Android has a different (wrong) interpretation?
Hide
Alex Miller added a comment -

I added a new clj-1472.patch that fixes a few minor details I didn't like about the prior patch. However, it is still essentially the same change so I have retained the original author attribution.

Show
Alex Miller added a comment - I added a new clj-1472.patch that fixes a few minor details I didn't like about the prior patch. However, it is still essentially the same change so I have retained the original author attribution.
Hide
Ghadi Shayban added a comment -

alex did you upload the right patch?

Show
Ghadi Shayban added a comment - alex did you upload the right patch?
Hide
Alex Miller added a comment -

Ah, no messed that up. Will fix.

Show
Alex Miller added a comment - Ah, no messed that up. Will fix.
Hide
Nicola Mometto added a comment -

Alex, it's probably worth making that a `^:once fn*`

Show
Nicola Mometto added a comment - Alex, it's probably worth making that a `^:once fn*`
Hide
Ghadi Shayban added a comment -

From Android:

Android doesn't run Java bytecode, it runs Dex bytecode. The dexdump output of what your class translates to is interesting.

Neither is the JVMS interesting. Android isn't a Java Virtual Machine. We follow the JLS, but not the JVMS (how could we, when we don't run Java bytecode). As such, all appeals against it are irrelevant. We try to be compatible to the spirit of the JVMS wrt/ Dex bytecode, but if your source isn't Java, there are no guarantees.

Now, the verifiers were (and probably still are) broken, even against our (pretty bad) spec, and regrettably we aren't very consistent. In Marshmallow, for example, a lot of the code we can't correctly verify wrt/ structured locking is rejected as a VerifyError, which is not in the spirit of the JVMS. In the next release, this will be relaxed, however, and postponed to an actual check while running the code.

Regrettably there isn't anything we can do about old releases, you'll have to work around any issues. I'll try to take a look at your class when I find the time.

Sounds like making a workaround in Clojure is the least of all evils.

Show
Ghadi Shayban added a comment - From Android:
Android doesn't run Java bytecode, it runs Dex bytecode. The dexdump output of what your class translates to is interesting. Neither is the JVMS interesting. Android isn't a Java Virtual Machine. We follow the JLS, but not the JVMS (how could we, when we don't run Java bytecode). As such, all appeals against it are irrelevant. We try to be compatible to the spirit of the JVMS wrt/ Dex bytecode, but if your source isn't Java, there are no guarantees. Now, the verifiers were (and probably still are) broken, even against our (pretty bad) spec, and regrettably we aren't very consistent. In Marshmallow, for example, a lot of the code we can't correctly verify wrt/ structured locking is rejected as a VerifyError, which is not in the spirit of the JVMS. In the next release, this will be relaxed, however, and postponed to an actual check while running the code. Regrettably there isn't anything we can do about old releases, you'll have to work around any issues. I'll try to take a look at your class when I find the time.
Sounds like making a workaround in Clojure is the least of all evils.
Hide
Alex Miller added a comment -

Added -2 patch with ^:once.

Show
Alex Miller added a comment - Added -2 patch with ^:once.
Hide
Alex Miller added a comment -

Our current belief is that Android is at fault here, so backlogging this for now.

Show
Alex Miller added a comment - Our current belief is that Android is at fault here, so backlogging this for now.
Hide
Ghadi Shayban added a comment -

GraalVM/native-image is also complaining about monitorenter/exit balancing. Fixed it by mimicing what javac does: https://github.com/ghadishayban/clojure/commit/8acb995853761bc48b62190fe7005b70da692510

Show
Ghadi Shayban added a comment - GraalVM/native-image is also complaining about monitorenter/exit balancing. Fixed it by mimicing what javac does: https://github.com/ghadishayban/clojure/commit/8acb995853761bc48b62190fe7005b70da692510
Hide
Alex Miller added a comment -

Ghadi, if that's a viable fix, I'm interested in a patch for that.

Show
Alex Miller added a comment - Ghadi, if that's a viable fix, I'm interested in a patch for that.
Hide
Ghadi Shayban added a comment -

If someone could assist me with Android testing infra, I'd be game.

In `javac`'s emission of synchronized, it installs a catch handler with the exception type "any". The linked commit catches Exception. If desired, we can emit the `any` handler (I don't know what that means yet exactly – Throwable + anything else in the future?) by calling GeneratorAdapter.visitTryCatch with `null` as the target class.

Show
Ghadi Shayban added a comment - If someone could assist me with Android testing infra, I'd be game. In `javac`'s emission of synchronized, it installs a catch handler with the exception type "any". The linked commit catches Exception. If desired, we can emit the `any` handler (I don't know what that means yet exactly – Throwable + anything else in the future?) by calling GeneratorAdapter.visitTryCatch with `null` as the target class.
Hide
Adam Clements added a comment - - edited

Have you tried applying the existing clj-1472-2.patch to see if that fixes GraalVM? I think we'd originally reached a place where people were happy with the attached patch (correct me if I'm wrong Alex) but it was decided that the JVM implementation is king so the problem was with Android for not conforming to that rather than with Clojure. It could be that decision is up for reconsideration if the same problem has reared its head elsewhere (I'd still like to see this patched myself).

This was an age ago now, but I remember trying all different combinations of clojure try/catch/finally/throw I could think of and not being able to get the emitted bytecode to conform to the spec for synchronised without altering clojure's code generation which was far too deep down to be practical - hence the above patch which relies on javac to generate the bytecode around the synchronisation instead of using monitor-enter and monitor-exit. I'd be concerned with your linked implementation that it might still generate wrong bytecode in some situations much like the existing implementation does even though it looks perfectly fine and apparently correct at the Clojure code level.

Show
Adam Clements added a comment - - edited Have you tried applying the existing clj-1472-2.patch to see if that fixes GraalVM? I think we'd originally reached a place where people were happy with the attached patch (correct me if I'm wrong Alex) but it was decided that the JVM implementation is king so the problem was with Android for not conforming to that rather than with Clojure. It could be that decision is up for reconsideration if the same problem has reared its head elsewhere (I'd still like to see this patched myself). This was an age ago now, but I remember trying all different combinations of clojure try/catch/finally/throw I could think of and not being able to get the emitted bytecode to conform to the spec for synchronised without altering clojure's code generation which was far too deep down to be practical - hence the above patch which relies on javac to generate the bytecode around the synchronisation instead of using monitor-enter and monitor-exit. I'd be concerned with your linked implementation that it might still generate wrong bytecode in some situations much like the existing implementation does even though it looks perfectly fine and apparently correct at the Clojure code level.
Hide
Ghadi Shayban added a comment -

I hadn't tried it but I'm sure clj-1472-2.patch would work from a feasability perspective. It has a perf downside: it can cause "profiling pollution" from the JVM's perspective and confuse lock optimization. (Most locks are uncontended)

I'd rather fix the emission to be more like javac, iff that works for Android.

Show
Ghadi Shayban added a comment - I hadn't tried it but I'm sure clj-1472-2.patch would work from a feasability perspective. It has a perf downside: it can cause "profiling pollution" from the JVM's perspective and confuse lock optimization. (Most locks are uncontended) I'd rather fix the emission to be more like javac, iff that works for Android.
Hide
Jason Felice added a comment -

I've tried the diff Ghadi linked to, but there are issues (including discarding the locked expression value). I fixed that, but after research, I've realized that:
1. This doesn't cover exceptions thrown by monitor-exit itself, which appears to be required. We can't recur in a catch, and we can't catch 'any', so we can't generate this bytecode with just Clojure constructs.
2. The bytecode emitted by Clojure's finally includes a store and a load uncovered by exception handling, so we can't just add exception handling inside the monitor-exit code. I'm pretty sure we can't remove the store and load.

So, unless I'm missing something, the only way to pass a structural locking check is to make locking a special form.

Show
Jason Felice added a comment - I've tried the diff Ghadi linked to, but there are issues (including discarding the locked expression value). I fixed that, but after research, I've realized that: 1. This doesn't cover exceptions thrown by monitor-exit itself, which appears to be required. We can't recur in a catch, and we can't catch 'any', so we can't generate this bytecode with just Clojure constructs. 2. The bytecode emitted by Clojure's finally includes a store and a load uncovered by exception handling, so we can't just add exception handling inside the monitor-exit code. I'm pretty sure we can't remove the store and load. So, unless I'm missing something, the only way to pass a structural locking check is to make locking a special form.
Hide
Adam Clements added a comment -

The already attached clj-1472-2.patch passes the structural locking tests.

After I did much exhaustive testing and comparing of generated bytecode back in 2014 this was the only viable approach I could find that didn't require instructing a new special form which reimplemented the poorly documented spec exactly to emit exactly the right bytecode, which seemed fragile to me.

I'd be interested to see some profiling numbers to see how big this claimed performance impact is, especially given how rare usage of the locking macro actually is in clojure (I could only find a few examples in the whole ecosystem when testing. Most people use the other concurrency primitives or java interop)

Show
Adam Clements added a comment - The already attached clj-1472-2.patch passes the structural locking tests. After I did much exhaustive testing and comparing of generated bytecode back in 2014 this was the only viable approach I could find that didn't require instructing a new special form which reimplemented the poorly documented spec exactly to emit exactly the right bytecode, which seemed fragile to me. I'd be interested to see some profiling numbers to see how big this claimed performance impact is, especially given how rare usage of the locking macro actually is in clojure (I could only find a few examples in the whole ecosystem when testing. Most people use the other concurrency primitives or java interop)
Hide
Alex Miller added a comment -

-3 patch rebases on master, no semantic changes, attribution retained

Show
Alex Miller added a comment - -3 patch rebases on master, no semantic changes, attribution retained
Hide
Jason Felice added a comment -

With clj-1472-3.patch, I can get builds working sometimes. Other times I get this error:

fatal error: com.oracle.svm.core.util.VMError$HostedError: Objects with relocatable pointers must be immutable              
        at com.oracle.svm.core.util.VMError.guarantee(VMError.java:85)                                                      
        at com.oracle.svm.hosted.image.NativeImageHeap.choosePartition(NativeImageHeap.java:492)                            
        at com.oracle.svm.hosted.image.NativeImageHeap.addObjectToBootImageHeap(NativeImageHeap.java:451)                   
        at com.oracle.svm.hosted.image.NativeImageHeap.addObject(NativeImageHeap.java:271)                                  
        at com.oracle.svm.hosted.image.NativeImageCodeCache.addConstantToHeap(NativeImageCodeCache.java:369)                
        at com.oracle.svm.hosted.image.NativeImageCodeCache.addConstantsToHeap(NativeImageCodeCache.java:356)               
        at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:882)                                  
        at com.oracle.svm.hosted.NativeImageGenerator.lambda$run$0(NativeImageGenerator.java:401)                           
        at java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(ForkJoinTask.java:1386)                             
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)                                                  
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)                                      
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)                                              
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)

I'm on Graal rc10, and the shortest code I can find to generate that error is:

(ns rep.core
  (:gen-class))

(defn- foo [s]
  (locking s
    42))

(defn -main
  [& args]
  (foo))

Locking on something which isn't a parameter, like a var, works around the issue.

This seems like it might be a Graal issue, so I'll report it there.

Show
Jason Felice added a comment - With clj-1472-3.patch, I can get builds working sometimes. Other times I get this error:
fatal error: com.oracle.svm.core.util.VMError$HostedError: Objects with relocatable pointers must be immutable              
        at com.oracle.svm.core.util.VMError.guarantee(VMError.java:85)                                                      
        at com.oracle.svm.hosted.image.NativeImageHeap.choosePartition(NativeImageHeap.java:492)                            
        at com.oracle.svm.hosted.image.NativeImageHeap.addObjectToBootImageHeap(NativeImageHeap.java:451)                   
        at com.oracle.svm.hosted.image.NativeImageHeap.addObject(NativeImageHeap.java:271)                                  
        at com.oracle.svm.hosted.image.NativeImageCodeCache.addConstantToHeap(NativeImageCodeCache.java:369)                
        at com.oracle.svm.hosted.image.NativeImageCodeCache.addConstantsToHeap(NativeImageCodeCache.java:356)               
        at com.oracle.svm.hosted.NativeImageGenerator.doRun(NativeImageGenerator.java:882)                                  
        at com.oracle.svm.hosted.NativeImageGenerator.lambda$run$0(NativeImageGenerator.java:401)                           
        at java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(ForkJoinTask.java:1386)                             
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)                                                  
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)                                      
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)                                              
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
I'm on Graal rc10, and the shortest code I can find to generate that error is:
(ns rep.core
  (:gen-class))

(defn- foo [s]
  (locking s
    42))

(defn -main
  [& args]
  (foo))
Locking on something which isn't a parameter, like a var, works around the issue. This seems like it might be a Graal issue, so I'll report it there.
Hide
Alex Miller added a comment -

Ok, please update. We’ve got probably a long while before the next release that might include this so good to get more feedback.

Show
Alex Miller added a comment - Ok, please update. We’ve got probably a long while before the next release that might include this so good to get more feedback.
Hide
Nicola Mometto added a comment -

I'll vote against making `locking` into a new special form if possible. It would make any analysis-related library forward-incompatible and likely cause pains in dependency upgrade

Show
Nicola Mometto added a comment - I'll vote against making `locking` into a new special form if possible. It would make any analysis-related library forward-incompatible and likely cause pains in dependency upgrade

People

Vote (5)
Watch (7)

Dates

  • Created:
    Updated: