ttl functions sometimes return nil

Description

If I (memoize/ttl) a function, near the threshold time the result from calling the memoized function will sometimes be nil.

I've attached a patch that exploits the problem in the unit tests.

Reproducing in the REPL is easy. Just call memoize/ttl fn a bunch of times and watch for nil results.

user=> (require '[clojure.core.memoize :as m])
nil

user=> (def f (m/ttl (constantly 1)))
#'user/f

user=> (dotimes [n 1000000] (when (nil? (f)) (println "nil f")))
nil f
nil f
nil f
nil f
nil f
nil

The problem seems to be that when clojure.core.cache/through gets called, the item hasn't expired, but when clojure.core.cache/lookup happens in build-memoizer, the expiration has passed.

Environment

None

Activity

Show:
Sean Corfield
March 2, 2018, 9:36 PM

Fixed in 0.7.1 release.

Sean Corfield
March 2, 2018, 9:28 PM

I thought I had this fixed in 0.7.0 with a single retry. However, it turns out that in pathological cases (slow machine, very high number of calls), the retry can also fail. So in 0.7.1 it retries multiple times (up to 10). In heavy load testing, I've only seen it hit a second retry very occasionally so this seems robust if still a nasty hack.

Sean Corfield
March 1, 2018, 9:17 AM

The path I took was the patch from Ryan, implementing Colin's suggestion. I've already fixed a similar issue directly in core.cache around TTLCache's odd expiry behavior and I had hoped that would also solve this issue, but it doesn't (I tested against 0.6.6-SNAPSHOT of core.cache).

I think pretty much everything suggested in this thread is a bit of a hack so revisiting this at a future date is definitely an option but, for now, I think the retry-on-not-found approach is the most expedient.

I intend to plough through the core.memoize issue backlog and close as many issues as I can over the next few days and then cut new releases of both core.cache and core.memoize.

import
May 10, 2017, 6:04 PM

Comment made by: pmonks

I hate to be "that guy", but are there any updates on when this will be fixed in an official release?

IMHO this is a pretty serious bug, and given that there are several patches floating around that address it (from & above, and also at least one Github fork) it seems like the only barrier to doing so is time & interest. Are those things the community can help with?

Josh Tilles
August 17, 2016, 5:01 PM

(or , of course) any thoughts on the strategy described above? Is it worth fleshing out?

Completed

Assignee

Sean Corfield

Reporter

Ryan Fowler

Patch

Code and Test