Help needed with a potential race condition

Hello there. I have a trading app connected to Interactive Brokers, and some weird race behaviour that I would welcome some opinion on. I’ve been able to reproduce the error through a stylised example below. I would welcome some help as to what I’m doing wrong, or better understanding why test-race-2 below is a bad pattern.

Thank you!!

(def data-deliverer (atom {}))
(def data-container (atom {}))

(defn request-historical-data!
"This is the function that calls Interactive brokers and returns some data, and delivers the promise on completion"
[req-id]
  (swap! data-container assoc req-id [1 2 3 4])
  (deliver (get @data-deliverer req-id) true))


(defn historical-request!
  "Start a request, update maps monitoring it, and return req-id on completion."
  []
  (let [req-id (rand-int 1000)]
    (swap! data-container assoc req-id [])
    (swap! data-deliverer assoc req-id (promise))
    (request-historical-data! req-id); sends the request to distant server
    (deref (get @data-deliverer req-id)); my assumption is this blocks
    req-id))

(defn test-race-1
  "This works and will log all the received quotes"
  []
  (let [req-id (historical-request!)]
    (doseq [hq (get @data-container req-id)]
      (println hq))))

(defn test-race-2
  "This does not work, it will print nothing, even though the request was processed and the data is present, but I presume came too late"
  []
  (doseq [hq (get @data-container (historical-request!))]
    (println hq)))

Nothing blocks in your code, it’s just that the last get returns nil. And there are no race conditions either - there’s no asynchronous code of any kind.

The reason for that is that in test-race-1, the call to get derefs data-container after (historical-request!), and in test-race-2 it happens before (historical-request!), so that get receives an old value that doesn’t have that request yet.

One other thing worth noting just in case is that you shouldn’t use code like this in a multithreaded environment. Operations on separate atoms are not synchronized in any way, and that could easily lead to proper race conditions.

Eugene, thank you very much, this makes complete sense, in my head things were evaluated right to left and I imagined the rest of the (get...) statement would not execute before historical-request returned, but I can see this was a big assumption, and the Clojure doc does say things are evaluated left to right anyway!

One other thing worth noting just in case is that you shouldn’t use code like this in a multithreaded environment. Operations on separate atoms are not synchronized in any way, and that could easily lead to proper race conditions

Isn’t this exactly what ref and dosync are for? There’s a whole STM engine in Clojure for just this scenario, right? :slight_smile:

https://clojure.org/reference/refs

https://clojuredocs.org/clojure.core/dosync

https://clojuredocs.org/clojure.core/ref

Yes. Sometimes it’s also possible to rewrite the code in such a way so that there’s no need for synchronization.

thanks both, I don’t really need synchronization between atoms. I have one atom that accumulates data, and one that just accumulates “data is ready” messages which tell the rest of my app that the first atom can be consumed. Welcome ideas as to how to do this better, but the less complexity the better (haven’t found need for e.g. async for instance)

Wouldn’t a chan be a better approach for this?

(ns historical-data
  (:require [clojure.core.async :refer [chan >! <!! go close!]]))

(defn request-historical-data!
  [req-id result-chan]
  (go
    (>! result-chan [1 2 3 4])
    (close! result-chan)))

(defn historical-request!
  []
  (let [req-id (rand-int 1000)
        result-chan (chan)]
    (request-historical-data! req-id result-chan)
    result-chan))

(defn test-race-1
  []
  (let [data (<!! (historical-request!))]
    (doseq [hq data]
      (println hq))))

(defn test-race-2
  []
  (doseq [hq (<!! (historical-request!))]
    (println hq)))

(defn -main [& args]
  (test-race-1)
  (test-race-2))