Concurrent posible problem in function get-proc

[fgerard]

I think I am having a memory leak an testing the usage of get-proc I think it should use "locking" like so:

(defn get-proc
  "Returns the Saxon Processor object, the thread-safe generator class for documents, 
  stylesheets, & XPaths. Creates & defs the Processor if not already created."
  {:tag Processor}
  []
  (locking get-proc
    (defonce ^:private p 
      (Processor. false)) 
    p))

This is because in a very heavy threading environment this can happen (example):

(defn f1 [m]
  (locking f1
    (println "Entrando a " m)
    (Thread/sleep 5000)
    (println "saliendo de " m)
    m))

(defn getx [] ; similar to get-proc
  (defonce ^:private c
    (f1 (rand-int 100)))
  c)

(doseq [n (range 0 3)]
  (future (getx)))

; you will see that f1 is invoked 3 times!

Best regards and thank you for this very helpful library.

Saludos

[pjt]

Thanks for catching this race condition & providing such a helpful reproducing example.

There are two ways I'm inclined to solve this. The first is a variant of your proposal, just a little more idiomatic: use delay & force for lazy initialization like so:

(def ^:private proc 
  (delay (Processor. false)))

(defn get-proc []
  (force proc))

This is a variant of your proposal because Delay objects get dereferenced through a call to a synchronized method, so calling (force proc) serializes access to the Processor constructor like your use of locking does.

The second way to solve this is not to attempt lazy initialization at all. Doing so was probably premature optimization.

Can I ask, do you happen to favor one or the other?

[fgerard]

I would go for just using defonce and omitting the laziness after all, you
are using the library right :)

On Tuesday, April 8, 2014, Perry Trolard notifications@github.com wrote:

Thanks for catching this race condition & providing such a helpful
reproducing example.

There are two ways I'm inclined to solve this. The first is a variant of
your proposal, just a little more idiomatic: use delay & force for lazy
initialization like so:

(def ^:private proc
(delay (Processor. false)))
(defn get-proc [](force proc))

This is a variant of your proposal because Delay objects get dereferenced
through a call to a synchronized method, so calling (force proc)serializes access to the Processor constructor like your use of
locking does.

The second way to solve this is not to attempt lazy initialization at all.
Doing so was probably premature optimization.

Can I ask, do you happen to favor one or the other?

Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-39926455
.

[image: fgerard.jpg]