Clojure

Locals cleared too aggressively on delay

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: Release 1.2
  • Component/s: None
  • Labels:
    None

Description

Steve Gilardi and I noticed some strange behaviour with local clearing in conjunction with delay:

(defn do-something [x]
(throw (Exception.)))

(defn clear-locals [x]
(delay (try (do-something x)
(catch Exception e
(println "argument value:" x)))))

(force (clear-locals :argument))
;; => argument value: nil

It seems the locals are getting cleared a little too aggressively here. I did some digging to discover that delay is a thin wrapper around an fn, but using an fn in this context (rather than a delay) does not trigger the problem. Also I found that throwing an exception directly inside the try block (rather than in a function called from the try block) did not trigger it.

I was able to reproduce in the new branch, the master branch, and 1.0.0, so it's not related to the more recent locals-clearing changes.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Assembla Importer added a comment -

technomancy said: [file:d6rA1a9ASr3RHaeJe5afGb]: repro case

Show
Assembla Importer added a comment - technomancy said: [file:d6rA1a9ASr3RHaeJe5afGb]: repro case
Hide
Permalink
Assembla Importer added a comment -

chouser@n01se.net said: 

((let [x :foo]
     (#^{:once true} fn* []
       (try (#(throw (Exception.)))
         (catch Exception e
           (println "x:" x))))))
</code></pre>
The same problem is visible when a finally clause uses a closed-over:
<pre><code>  ((let [x :foo]
     (#^{:once true} fn* []
       (try (#(throw (Exception.)))
         (finally
           (println "x:" x))))))

Both of these print x: nil when they should print x: :foo

I think the problem is that, because the last expr in a try block produces the value that will be returned, it is compiled in a "return" context (a.k.a. tail position), therefore the locals are cleared before calling the final function.

But if that final call throws an exception, you end up in the catch clause with your locals cleared.

emitClearLocals protects against this problem by using localsUsedInCatchFinally, but emitClearCloses (used only when :once is true) does not.

The solution is so very far beyond me. I guess the simplest might be to skip clearing of closed-overs that are used in catch/finally clauses, but I don't think those are currently tracked. It looks like closeOver could be made to track these as well, but I'm lost as to the relationships between the various instances in that code.

Show
Assembla Importer added a comment - chouser@n01se.net said: 
((let [x :foo]
     (#^{:once true} fn* []
       (try (#(throw (Exception.)))
         (catch Exception e
           (println "x:" x))))))
</code></pre>
The same problem is visible when a finally clause uses a closed-over:
<pre><code>  ((let [x :foo]
     (#^{:once true} fn* []
       (try (#(throw (Exception.)))
         (finally
           (println "x:" x))))))
Both of these print x: nil when they should print x: :foo I think the problem is that, because the last expr in a try block produces the value that will be returned, it is compiled in a "return" context (a.k.a. tail position), therefore the locals are cleared before calling the final function. But if that final call throws an exception, you end up in the catch clause with your locals cleared. emitClearLocals protects against this problem by using localsUsedInCatchFinally, but emitClearCloses (used only when :once is true) does not. The solution is so very far beyond me. I guess the simplest might be to skip clearing of closed-overs that are used in catch/finally clauses, but I don't think those are currently tracked. It looks like closeOver could be made to track these as well, but I'm lost as to the relationships between the various instances in that code.
Hide
Permalink
Assembla Importer added a comment -

danlarkin said: Kevin Downey and I just stumbled on this problem again. Here's our smallest repro case:

(defn throwsomething [] (throw (Exception.)))

(defn foo [bar]
  @(delay (try
           (throwsomething)
           (catch Exception e
             (nil? bar)))))
</code></pre>

We came up with two workarounds, one is to wrap everything inside the delay in a let capturing the "bar" scope, like this:

<pre><code>
(defn foo [bar]
  @(delay (let [bar bar]
            (try
             (throwsomething)
             (catch Exception e
               (nil? bar))))))
</code></pre>

and the other is to not have the exception-throwing call in the tail position, like this:

<pre><code>
(defn foo [bar]
  @(delay (try
           (let [t (throwsomething)]
             t)
           (catch Exception e
             (nil? bar)))))
Show
Assembla Importer added a comment - danlarkin said: Kevin Downey and I just stumbled on this problem again. Here's our smallest repro case: 
(defn throwsomething [] (throw (Exception.)))

(defn foo [bar]
  @(delay (try
           (throwsomething)
           (catch Exception e
             (nil? bar)))))
</code></pre>

We came up with two workarounds, one is to wrap everything inside the delay in a let capturing the "bar" scope, like this:

<pre><code>
(defn foo [bar]
  @(delay (let [bar bar]
            (try
             (throwsomething)
             (catch Exception e
               (nil? bar))))))
</code></pre>

and the other is to not have the exception-throwing call in the tail position, like this:

<pre><code>
(defn foo [bar]
  @(delay (try
           (let [t (throwsomething)]
             t)
           (catch Exception e
             (nil? bar)))))
Hide
Permalink
Assembla Importer added a comment -

richhickey said: (In [[r:93fecbd825c26e2570f8449cd64d0df0cc520c1d]]) fold closes clearing into the path system, fixes #232

Branch: master

Show
Assembla Importer added a comment - richhickey said: (In [[r:93fecbd825c26e2570f8449cd64d0df0cc520c1d]]) fold closes clearing into the path system, fixes #232 Branch: master

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.