onyx-platform / onyx-dashboard

Dashboard for the Onyx distributed processing system
http://www.onyxplatform.org/
Eclipse Public License 1.0
76 stars 26 forks source link

refresh-deployments-watch task not cancellable #60

Closed mariusz-jachimowicz-83 closed 8 years ago

mariusz-jachimowicz-83 commented 8 years ago
; when
; refresh-fut       (future (tenancy/refresh-deployments-watch send-f conn deployments))
; then
(future-cancel (:refresh-fut component))

is not cancelling the future. It is due to catch of

(catch InterruptedException ie

inside refresh-deployments-watch. So this job is still running after reload code in repl.

You can easly see this in plain repl. For instance when you run

(defn refresh-deployments-watch []
  (loop []
    (println "---------")
    (println "i-a") 
    (try
      (println "i-b")    
      (when-not (Thread/interrupted)
        (do (println "i-c") 
            (Thread/sleep 1000))
        )
      ; catching exceptions prevents future cancellation
      (catch InterruptedException ie
        (println "Shutting down refresh deployments watch"))
      (catch Throwable t
        (println t "Error watching deployments"))
      )
    (recur)))

; examine cancellation
(let [f1 (future (refresh-deployments-watch))]
      (Thread/sleep 5000)
      (future-cancel f1))

this job is always running, it can't be cancelled, it is always recuring While without handling exception all is ok

(defn refresh-deployments-watch []
  (loop []
    (println "---------")
    (println "i-a") 
    (try
      (println "i-b")    
      (when-not (Thread/interrupted)
        (do (println "i-c") 
            (Thread/sleep 1000))
        )
      ; (catch InterruptedException ie
      ;   (println "Shutting down refresh deployments watch"))
      ; (catch Throwable t
      ;   (println t "Error watching deployments"))
      )
    (recur)))

; examine cancellation
(let [f1 (future (refresh-deployments-watch))]
      (Thread/sleep 5000)
      (future-cancel f1))
lbradstreet commented 8 years ago

Thanks. This is an excellent point, which may show a flaw in my understanding of how InterruptedExceptions work.

mariusz-jachimowicz-83 commented 8 years ago

I am working on solution for this.

gardnervickers commented 8 years ago

@mariusz-jachimowicz-83 I think you'd want something like:

(while (not (Thread/interrupted))
  ...)
lbradstreet commented 8 years ago

See the bottom of this page for the right way to deal with this. Swallowing was incorrect. Good catch.

http://www.yegor256.com/2015/10/20/interrupted-exception.html

mariusz-jachimowicz-83 commented 8 years ago

Manually closed after PR merge