search

leonoel / cloroutine

Coroutine support for clojure
Eclipse Public License 2.0
230 stars 10 forks source link

Add `locking` support #12

Open darkleaf opened 4 years ago

darkleaf commented 4 years ago

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
leonoel commented 4 years ago

why fake ?

darkleaf commented 4 years ago

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.

leonoel commented 4 years ago

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))

darkleaf commented 4 years ago

Agree

darkleaf commented 4 years ago

thanks

leonoel commented 4 years ago

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.

darkleaf commented 4 years ago 
   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 commented 4 years ago

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