Cache.asMap().size() may return a negative value

[ben-manes]

My multithreaded thrashing tests failed with Guava due to size() returning a negative value. The size is used for verifying that the toArray() methods are threadsafe (this was a common bug until JDK6 rewrote AbstractCollection's to be tolerant to races). The lines in question are,

(cache, key) -> cache.asMap().keySet().toArray(new Object[cache.asMap().size()]),
(cache, key) -> cache.asMap().values().toArray(new Object[cache.asMap().size()]),
(cache, key) -> cache.asMap().entrySet().toArray(new Entry[cache.asMap().size()]),

This results in a NegativeArraySizeException unless worked around using Math.max(0, cache.asMap().size()) which is now done in the guava fixture. I'm not sure why this occurs, and only happened after tweaking some JVM args. Regardless this failure was reproducible and is simple to fix.

java.lang.NegativeArraySizeException
at com.github.benmanes.caffeine.cache.MultiThreadedTest.lambda$new$280(MultiThreadedTest.java:142)
at com.github.benmanes.caffeine.cache.MultiThreadedTest$$Lambda$29/584234975.accept(Unknown Source)
at com.github.benmanes.caffeine.testing.Threads$Thrasher.run(Threads.java:149)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at com.github.benmanes.caffeine.testing.ConcurrentTestHarness.lambda$timeTasks$344(ConcurrentTestHarness.java:100)
at com.github.benmanes.caffeine.testing.ConcurrentTestHarness$$Lambda$61/1027825150.run(Unknown Source)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)

[kluever]

Are your caches really big? I see we do some summing up of segment sizes in a loop - maybe we are overflowing Integer.MAX_VALUE?

[ben-manes]

The saturated cast should handle that. Both ConcurrentHashMap
implementations do a negative check, so it sounds like an expected race
even though I don't know why it might occur.
On Jul 15, 2015 7:45 AM, "Kurt Alfred Kluever" notifications@github.com
wrote:

Are your caches really big? I see we do some summing up of segment
sizes in a loop - maybe we are overflowing Integer.MAX_VALUE?

—
Reply to this email directly or view it on GitHub
#2108 (comment).

[lowasser]

"which is now done in the guava fixture" What does this mean? It's not 100% clear what fix you have in mind. Is it just a simple Math.max in the size method?

[ben-manes]

Yes, its that simple. In longSize() use return Math.max(0, sum) which will cover all of the size() methods. This is essentially what I do in my fixture, which is used to run all my tests against both Caffeine and Guava implementations to find bugs / check compatibility.

Again I'm not sure why this occurs from reading the code, but I was able to reproduce the exception (its racy so takes a few tries). The fix is safe and non-invasive, so easier to resolve it there than dig into the race causing it.

[kevinb9n]

Is it enough to max(0, entireSize) or should it be done to each individual segment count?

[ben-manes]

Either is fine with me. Since the size is an estimate, rather than exact, I'm not overly concerned either way. The long avoids overflow so your idea would be for better precision.

[lowasser]

Patch submitted internally; that should make it out into the next RC.