Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hangs in clojure #75

Open
mobileink opened this issue Jan 3, 2021 · 14 comments
Open

hangs in clojure #75

mobileink opened this issue Jan 3, 2021 · 14 comments

Comments

@mobileink
Copy link

Hi, I'm trying to get re-graph working in a clojure command-line app. Following the examples, nothing happens, and the program will not exit, even if I add (shutdown-agents) at the end. E.g.

(defn -main
  [& args]

  (println "entering main")
  (re-graph/init {:ws-url   "ws://localhost:3086/graphql-ws"
                  :http-url "http://localhost:3085/graphql"})
  (println "exiting main")
  (shutdown-agents))

Running it:

$ clojure -M -m mina.repl
entering main
exiting main
...hangs...

Adding the example code to send a query has no effect. What am I doing wrong?

@oliyh
Copy link
Owner

oliyh commented Jan 3, 2021

Hi,

You should call re-graph/destroy when you have finished with it. I wouldn't expect it to hang forever without this though. Which http client are you using, hato (default) or gniazdo? (See readme for details).

Cheers

@mobileink
Copy link
Author

There's nothing in the readme about re-graph/destroy, and I don't see any other documentation. I'm using hato.

@oliyh
Copy link
Owner

oliyh commented Jan 3, 2021

Yes I realised when I replied that destroy is undocumented, I have added that.

I will try to replicate your issue, in the meantime are you able to take a thread dump while it is hanging? That would help greatly.

Thanks
Oliy

@oliyh
Copy link
Owner

oliyh commented Feb 2, 2022

Closing this as no update, please let me know if you still face issues.

Cheers

@oliyh oliyh closed this as completed Feb 2, 2022
@oliyh
Copy link
Owner

oliyh commented Mar 16, 2022

Was able to reproduce and found this non-daemon thread preventing shutdown:

"pool-2-thread-1" #16 prio=5 os_prio=0 cpu=235.02ms elapsed=41.72s tid=0x00007f11524f3000 nid=0x67fb4 waiting on condition  [0x00007f11251fc000]
   java.lang.Thread.State: WAITING (parking)
    at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
    - parking to wait for  <0x000000070c400170> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
    at java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:194)
    at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await([email protected]/AbstractQueuedSynchronizer.java:2081)
    at java.util.concurrent.LinkedBlockingQueue.take([email protected]/LinkedBlockingQueue.java:433)
    at java.util.concurrent.ThreadPoolExecutor.getTask([email protected]/ThreadPoolExecutor.java:1054)
    at java.util.concurrent.ThreadPoolExecutor.runWorker([email protected]/ThreadPoolExecutor.java:1114)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run([email protected]/ThreadPoolExecutor.java:628)
    at java.lang.Thread.run([email protected]/Thread.java:829)

Turns out it was the executor from here:
https://github.com/day8/re-frame/blob/master/src/re_frame/interop.clj#L26

Going to talk to re-frame devs to find out more. Short term workaround is to kill that executor.

@lowecg
Copy link

lowecg commented Nov 1, 2024

@oliyh Hi Oli - did you ever get a resolution to this?

Running clj -X:test from the CLI hangs at the end of a test. A thread dump from VisualVM reveals a stack similar to the one above.

"pool-1-thread-1" #31 [40707] prio=5 os_prio=31 cpu=16.75ms elapsed=487.53s tid=0x0000000126bd4200 nid=40707 waiting on condition  [0x000000016f2e2000]
   java.lang.Thread.State: WAITING (parking)
        at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
        - parking to wait for  <0x000000040105d1c0> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
        at java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:371)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionNode.block([email protected]/AbstractQueuedSynchronizer.java:519)
        at java.util.concurrent.ForkJoinPool.unmanagedBlock([email protected]/ForkJoinPool.java:3780)
        at java.util.concurrent.ForkJoinPool.managedBlock([email protected]/ForkJoinPool.java:3725)
        at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await([email protected]/AbstractQueuedSynchronizer.java:1712)
        at java.util.concurrent.LinkedBlockingQueue.take([email protected]/LinkedBlockingQueue.java:435)
        at java.util.concurrent.ThreadPoolExecutor.getTask([email protected]/ThreadPoolExecutor.java:1070)
        at java.util.concurrent.ThreadPoolExecutor.runWorker([email protected]/ThreadPoolExecutor.java:1130)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run([email protected]/ThreadPoolExecutor.java:642)
        at java.lang.Thread.runWith([email protected]/Thread.java:1596)
        at java.lang.Thread.run([email protected]/Thread.java:1583)

   Locked ownable synchronizers:
        - None

@lowecg
Copy link

lowecg commented Nov 1, 2024

For now, I have a what can be charitably called a "workaround":

(:require 
    [clojure.test :refer [is, testing, deftest, use-fixtures]
    [re-frame.interop])

(def get-executor
  (let [executor-var (resolve 're-frame.interop/executor)]
    #(var-get executor-var)))

(defn shutdown-executor []
  (.shutdown (get-executor)))

(use-fixtures :once
              (fn [f]
                (f)  ; runs all tests
                (shopify-api/shutdown-executor)))

@oliyh
Copy link
Owner

oliyh commented Nov 1, 2024

Hi @lowecg,

Well done! This would probably be a great contribution to re-frame-test - if they accept it then could update martian to use it. If not then I'm also happy to accept this into martian as a test fixture.

Cheers

@oliyh oliyh reopened this Nov 1, 2024
@oliyh
Copy link
Owner

oliyh commented Nov 14, 2024

Hi @lowecg

Did you get a chance to discuss this with the re-frame-test maintainers? I could open an issue and point them to this thread if not.

Cheers

@lowecg
Copy link

lowecg commented Nov 15, 2024 via email

@lowecg
Copy link

lowecg commented Nov 15, 2024

EDIT: this turned out to be unnecessary - re-graph works fine in Lambda and SnapStart without having manipulate the executor.

There is another angle to this functionality where I had to resurrect the executor.

Problem:
When using re-graph from a Lambda function, I needed to shutdown the executor to prevent process hang; but Lambdas are often reused and, once the executor has been shutdown, reuse is not possible.

Solution:
Reset the executor. Call reset at the start of the lambda and shutdown at the end, and that seems to allow this use case.

(defn reset-executor!
  "Resets the executor used by re-frame (used by re-graph for GraphQL execution).

   This allows repeat executions of Lambda"
  []
  (let [current-executor (get-executor)]
    (when (and current-executor (.isShutdown ^ExecutorService current-executor))
      (alter-var-root #'re-frame.interop/executor
                      (fn [_] (Executors/newSingleThreadExecutor))))))

(defn shutdown-executor
  "Shuts down the executor used by re-frame (used by re-graph for GraphQL execution).

   Warning: once called, further GraphQL interactions will not be possible unless you call reset-executor!.

   This is necessary as the non-daemon executor is not shut down by default and will cause short-lived processes (e.g. Lambda or tests run from the CLI) to hang."
  []
  (.shutdown ^ExecutorService (get-executor)))

@lowecg
Copy link

lowecg commented Nov 17, 2024

I've raised a PR for this with re-frame, so 🤞

@lowecg
Copy link

lowecg commented Dec 3, 2024

@oliyh would you consider this issue closed given that we have a workaround and the re-frame team (understandably) want to keep the executor internal?

@oliyh
Copy link
Owner

oliyh commented Dec 4, 2024

Following the other thread - let's see what happens there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants