Effective dispatch value is incorrectly cached for `nil`
camsaul opened this issue · 1 comments
camsaul commented
This leads to a stack overflow:
(let [mf* (atom nil)
mf (-> (m/multifn
(m/standard-multifn-impl
(m/thread-last-method-combination)
(m/multi-default-dispatcher identity)
(m/standard-method-table)))
(m/add-primary-method :default (fn [_next-method m] m))
(m/add-primary-method nil (fn [_next-method m]
(@mf* :default))))]
(reset! mf* mf)
(try
(mf nil)
(catch Throwable e
(pprint/pprint (update-vals (deref (.. mf impl cache atomm)) meta))
(throw e))))
;; => {:default {:dispatch-value :default}, nil {:dispatch-value :default}}
;; => StackOverflowError
The nil
method is incorrectly getting cached for both nil
and :default
. This is a race condition/caching bug, since the same issue doesn't happen if you use an uncached multifn.
If you invoke (mf :default)
before you invoke (mf nil)
, the :default
method gets cached for both nil
and :default
. This means there is no stack overflow, but the results are still wrong.
This is probably an issue specifically with handling falsey dispatch values.
camsaul commented
Failing test
(deftest nil-dispatch-values-test
(doseq [order [[nil :default]
[:default nil]]]
(testing (pr-str order)
(let [mf* (atom nil)
mf (-> (m/multifn
(m/standard-multifn-impl
(m/thread-last-method-combination)
(m/multi-default-dispatcher identity)
(m/standard-method-table)))
(m/add-primary-method :default (fn [_next-method m] :default))
(m/add-primary-method nil (fn [_next-method m]
(@mf* :default)
nil)))]
(reset! mf* mf)
(doseq [x order]
(is (= x
(mf x))))))))