cloroutine icon indicating copy to clipboard operation
cloroutine copied to clipboard

Add `locking` support

Open darkleaf opened this issue 5 years ago • 8 comments

I've been trying to debug coroutines with hashp and I've got an exception. Cloroutine doesn't handle locking form.

I use latest version of cloroutine.

I think cloroutine should have fake locking implementation.

Caused by java.lang.IllegalArgumentException
No matching clause: :monitor-enter

                 impl.cljc:  453  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                 impl.cljc:  166  cloroutine.impl$fn__3488$collect__3570/doInvoke
               RestFn.java:  497  clojure.lang.RestFn/invoke
                 impl.cljc:  530  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                 impl.cljc:  418  cloroutine.impl$fn__3488$add_branch__3673/invoke
                 impl.cljc:  547  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                  AFn.java:  156  clojure.lang.AFn/applyToHelper
                  AFn.java:  144  clojure.lang.AFn/applyTo
                  core.clj:  667  clojure.core/apply
                  core.clj:  660  clojure.core/apply
                 impl.cljc:  405  cloroutine.impl$fn__3488$add_bindings__3663/doInvoke
               RestFn.java:  467  clojure.lang.RestFn/invoke
                 impl.cljc:  521  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                  AFn.java:  156  clojure.lang.AFn/applyToHelper
                  AFn.java:  144  clojure.lang.AFn/applyTo
                  core.clj:  667  clojure.core/apply
                  core.clj:  660  clojure.core/apply
                 impl.cljc:  405  cloroutine.impl$fn__3488$add_bindings__3663/doInvoke
               RestFn.java:  467  clojure.lang.RestFn/invoke
                 impl.cljc:  521  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                 impl.cljc:  399  cloroutine.impl$fn__3488$add_bindings__3663/doInvoke
               RestFn.java:  467  clojure.lang.RestFn/invoke
                 impl.cljc:  521  cloroutine.impl$fn__3488$add_breaking__3683/invoke
                 impl.cljc:  593  cloroutine.impl$fn__3488$fn__3710/invoke
                 impl.cljc:  694  cloroutine.impl$compile/invokeStatic
                 impl.cljc:  691  cloroutine.impl$compile/invoke
                 core.cljc:   33  cloroutine.core$cr/invokeStatic
                 core.cljc:    3  cloroutine.core$cr/doInvoke
               RestFn.java:  146  clojure.lang.RestFn/applyTo
                  Var.java:  705  clojure.lang.Var/applyTo
             Compiler.java: 6993  clojure.lang.Compiler/macroexpand1
             Compiler.java: 7093  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java: 3888  clojure.lang.Compiler$InvokeExpr/parse
             Compiler.java: 7109  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 7095  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java: 6120  clojure.lang.Compiler$BodyExpr$Parser/parse
             Compiler.java: 5467  clojure.lang.Compiler$FnMethod/parse
             Compiler.java: 4029  clojure.lang.Compiler$FnExpr/parse
             Compiler.java: 7105  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 7095  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java:   38  clojure.lang.Compiler/access$300
             Compiler.java: 6384  clojure.lang.Compiler$LetExpr$Parser/parse
             Compiler.java: 7107  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 7095  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java: 6120  clojure.lang.Compiler$BodyExpr$Parser/parse
             Compiler.java: 5467  clojure.lang.Compiler$FnMethod/parse
             Compiler.java: 4029  clojure.lang.Compiler$FnExpr/parse
             Compiler.java: 7105  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 7095  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java: 3104  clojure.lang.Compiler$MapExpr/parse
             Compiler.java: 6797  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java:  594  clojure.lang.Compiler$DefExpr$Parser/parse
             Compiler.java: 7107  clojure.lang.Compiler/analyzeSeq
             Compiler.java: 6789  clojure.lang.Compiler/analyze
             Compiler.java: 6745  clojure.lang.Compiler/analyze
             Compiler.java: 7181  clojure.lang.Compiler/eval
             Compiler.java: 7636  clojure.lang.Compiler/load
                      REPL:    1  darkleaf.effect.core/eval19584
                      REPL:    1  darkleaf.effect.core/eval19584
             Compiler.java: 7177  clojure.lang.Compiler/eval
             Compiler.java: 7132  clojure.lang.Compiler/eval
                  core.clj: 3214  clojure.core/eval
                  core.clj: 3210  clojure.core/eval
                  main.clj:  437  clojure.main/repl/read-eval-print/fn
                  main.clj:  437  clojure.main/repl/read-eval-print
                  main.clj:  458  clojure.main/repl/fn
                  main.clj:  458  clojure.main/repl
                  main.clj:  368  clojure.main/repl
               RestFn.java: 1523  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   79  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:   55  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:  142  nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn
                  AFn.java:   22  clojure.lang.AFn/run
               session.clj:  171  nrepl.middleware.session/session-exec/main-loop/fn
               session.clj:  170  nrepl.middleware.session/session-exec/main-loop
                  AFn.java:   22  clojure.lang.AFn/run
               Thread.java:  835  java.lang.Thread/run

darkleaf avatar Dec 23 '19 18:12 darkleaf

why fake ?

leonoel avatar Dec 23 '19 18:12 leonoel

Coroutine may be broken(suspended) inside locking form. It may make some problems or not.

If you can add real implementation it will be cool.

darkleaf avatar Dec 23 '19 19:12 darkleaf

At first glance I'd say the benefits of having it outweight the risks. BTW threads give you the same footgun, nothing prevents you to write (locking x (Thread/sleep Long/MAX_VALUE))

leonoel avatar Dec 23 '19 20:12 leonoel

Agree

darkleaf avatar Dec 23 '19 20:12 darkleaf

thanks

darkleaf avatar Dec 29 '19 15:12 darkleaf

Actually, the fix doesn't work on latest runtimes, because Unsafe/monitorEnter and Unsafe/monitorExit have been ditched.

We can't rely on JVM opcodes monitorEnter/monitorExit because the JVM spec allows runtimes to throw IllegalMonitorStateException if a monitor is not properly balanced at the method level.

I reopen this one until we find an acceptable workaround.

leonoel avatar Dec 29 '19 16:12 leonoel

   No matching method monitorExit found taking 1 args for class
   sun.misc.Unsafe

            Reflector.java:  127  clojure.lang.Reflector/invokeMatchingMethod
            Reflector.java:  102  clojure.lang.Reflector/invokeInstanceMethod
             register.cljc:   75  publicator.core.use_cases.user.register$process$cr23192_block_11__23219/invoke
                 impl.cljc:   60  cloroutine.impl$coroutine$fn__7323/invoke

Clojure 1.10.1, Java 12.0.2

darkleaf avatar Dec 29 '19 16:12 darkleaf

@leonoel can you also fix this issue? I need any debugging tool. Cider debugger doesn't handle macros that uses &env argument.

darkleaf avatar Jan 17 '20 17:01 darkleaf