search

scgilardi / slingshot

Enhanced try and throw for Clojure leveraging Clojure's capabilities
654 stars 28 forks source link

throw+ from a delay in Clojure 1.4 results in cleared locals on future deref attempts #28

Closed charles-dyfis-net closed 10 years ago

charles-dyfis-net commented 12 years ago

The following may boil down to a bug in Clojure upstream. However, I'm unable to reproduce it without the use of Slingshot, and don't understand the intricacies of Slingshot's internals well enough to generate a non-slingshot-dependent reproducer to be filed upstream. Hence:

(use 'slingshot.slingshot)

(defn repro-bug [ref]
  (delay
    (cond
      (nil? ref) :nil
      (= :throw @ref) (throw+ "Throwing per request")
      :else @ref)))

(def repro-bug-atom (atom :throw))
(def repro-bug-inst (repro-bug repro-bug-atom))
(deref repro-bug-inst) ;; correctly throws "Throwing per request"

(reset! repro-bug-atom "Value")
(deref repro-bug-inst) ;; incorrectly returns :nil

If using (throw (Exception. "Throwing per request")) rather than (throw+ "Throwing per request"), the second deref returns "Value".

pjstadig commented 10 years ago

I know this is a rather old issue, but I thought I'd shed some light on it. The behavior here is the result of the interaction between two things: 1) in Clojure 1.4 (and 1.5) there was a bug in the delay implementation where it did not cache the result of the body of the delay when an exception occurred, and 2) slingshot's environment capture feature is interacting with locals clearing.

  1. In general you should not expect this code to do anything different the second time around. A delay is supposed to memoize the result of executing its body so that subsequent derefs return that cached value. In Clojure 1.4 it would reexecute a delay if an exception occured during the first deref. This bug is fixed in 1.6 (http://dev.clojure.org/jira/browse/CLJ-1175), so running this example on 1.6 will cause an exception to get thrown during the second deref instead of returning either "Value" or :nil (both of which are incorrect results).
  2. The reason this example is returning :nil instead of "Value" (like the code does when you change the throw+ to a throw (which is still incorrect behavior)) is because of slingshot's environment capture. Slingshot is capturing ref into its throw context. I'm a little fuzzy on exactly what is happening here, but for some reason slingshot's environment capture is interacting with locals clearing so that the second time through the delay ref is actually nil. When you use a plain throw you don't get this bad locals clearing interaction. Disabling slingshot's environment capture makes the behavior of throw and throw+ the same.

Conclusion: this behavior is "fixed" in 1.6, it is mostly a bug in Clojure's delay, because it never should have executed the body of the delay more than once, but also slingshot's environment capture is contributing to the weirdness here (see https://github.com/scgilardi/slingshot/issues/36).

scgilardi commented 10 years ago

closing this along with issue #36