Clojure

Agent sends consume heap

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.3
  • Fix Version/s: Release 1.4
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Ok

Description

Simple demonstration:

(defn update [state]
  (send *agent* update)
  (inc state))

(def a (agent 1))

(send a update)

On Clojure 1.2.1, this runs forever with no problem. On 1.3.0, it throws "java.lang.OutOfMemoryError: Java heap space."

The problem appears to be clojure.core/binding-conveyor-fn: each send creates a new Var binding frame, and nested send creates an infinite stack of frames.

Also discussed at https://groups.google.com/d/topic/clojure/1qUNPZv3OYA/discussion

Activity

Hide
Tassilo Horn added a comment -

I've just checked the code: send uses binding (which calls push-thread-bindings, creating a new Frame with non-null prev) inside which the binding-conveyor-fn captures the current frame. When the binding-conveyor-fn runs, the old Frame is restored, and since the wrapped function sends again, a new thread-binding is pushed on top of that.

Hm, one way to get rid of the issue was not to capture the "real" current Frame in the binding-conveyor-fn but only a shallow copy, i.e., a Frame with the same bindings but no prev. I've tried that out, all tests still pass, and the example above won't grow heap without bounds. Patch attached.

Show
Tassilo Horn added a comment - I've just checked the code: send uses binding (which calls push-thread-bindings, creating a new Frame with non-null prev) inside which the binding-conveyor-fn captures the current frame. When the binding-conveyor-fn runs, the old Frame is restored, and since the wrapped function sends again, a new thread-binding is pushed on top of that. Hm, one way to get rid of the issue was not to capture the "real" current Frame in the binding-conveyor-fn but only a shallow copy, i.e., a Frame with the same bindings but no prev. I've tried that out, all tests still pass, and the example above won't grow heap without bounds. Patch attached.
Tassilo Horn made changes -
Field Original Value New Value
Attachment 0001-Only-capture-a-shallow-copy-of-the-current-Frame-in-.patch [ 10749 ]
Rich Hickey made changes -
Patch Code [ 10001 ]
Fix Version/s Approved Backlog [ 10034 ]
Fix Version/s Release 1.4 [ 10040 ]
Hide
Stuart Sierra added a comment -

Screened.

Show
Stuart Sierra added a comment - Screened.
Stuart Sierra made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Sierra made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
Hide
Tassilo Horn added a comment -

My fix had some small issue as David Miller pointed out correctly in

https://github.com/clojure/clojure/commit/12f07da889819bc5613546ec223e97ac27c86dbf#commitcomment-867608

Attached is a patch that fixes the quirk.

Show
Tassilo Horn added a comment - My fix had some small issue as David Miller pointed out correctly in https://github.com/clojure/clojure/commit/12f07da889819bc5613546ec223e97ac27c86dbf#commitcomment-867608 Attached is a patch that fixes the quirk.
Tassilo Horn made changes -
Assignee Stuart Sierra [ stuart.sierra ]
Status Resolved [ 5 ] Reopened [ 4 ]
Resolution Completed [ 1 ]
Hide
Tassilo Horn added a comment -

Here's the fix for the fix.

Show
Tassilo Horn added a comment - Here's the fix for the fix.
Tassilo Horn made changes -
Stuart Sierra made changes -
Resolution Completed [ 1 ]
Status Reopened [ 4 ] Resolved [ 5 ]
Stuart Halloway made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: