[
https://issues.apache.org/jira/browse/COLLECTIONS-776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17419514#comment-17419514
]
ckurz commented on COLLECTIONS-776:
-----------------------------------
I believe I found the bug, the class
{{org.apache.commons.collections4.map.PassiveExpiringMap}} does not provide an
appropriate set when the Method
{{org.apache.commons.collections4.map.PassiveExpiringMap#keySet}} is called
which reflects the intended behavior. Similar methods probably have similar
problems.
Does this need support? Lets look into the Documentation
({{java.util.SortedMap#keySet}}):
{quote}[...] The set is backed by the map, so changes to the map are reflected
in the set, and vice-versa. [...] The set supports element removal, which
removes the corresponding mapping from the map, via the Iterator.remove,
Set.remove, removeAll, retainAll, and clear operations. [...]
{quote}
It seems to me that the class is not in line with the documentation since it
possible to get a key-set which holds keys which are will not be retrievable
from the map. See code example below.
Fix:
I suggest we return a Set decorated which has the same functionality as the
{{PassiveExpiringMap}} so something like a {{PassiveExpiringSet}} if such a
thing exists. We should also add this functionality for the
{{PassiveExpiringMap#values}} method and similar cases.
One side note, this will probably get a little bit messy, there is some
mutability involved which will fail in case of the multithreading and
concurrency. Also keeping a key-set of a map in sync with the actual map
probaply isnt the best idea :), even though I have some ideas how to adress
this.
The following code reflects a bit more the problem I try to point out. It
actualy occurs in line 18 (final Set<String> keySet = m.keySet();):
{code:java}
import org.apache.commons.collections4.map.PassiveExpiringMap;
import java.text.SimpleDateFormat;
import java.time.Instant;
import java.util.*;
import java.util.concurrent.TimeUnit;
public class Test {
private static final SimpleDateFormat SDF = new
SimpleDateFormat("HH:mm:ss");
public static void main(String[] args) throws InterruptedException {
final PassiveExpiringMap<String, Object> m = new
PassiveExpiringMap<>(3, TimeUnit.SECONDS);
final String key = UUID.randomUUID().toString();
m.put(key, "");
final Set<String> keySet = m.keySet();
final Instant before = Instant.now();
while (Instant.now().isBefore(before.plusSeconds(5))) {
logWithTimestamp("Keys in wrapped map: " + String.join(", ",
keySet));
try {
Thread.sleep(1000);
} catch (InterruptedException ignored) {
}
}
}
private static void logWithTimestamp(String string) {
final Date resultdate = new Date(System.currentTimeMillis());
System.out.println(Thread.currentThread().getName() + " [" +
SDF.format(resultdate) + "] " + string);
}
}
{code}
Any thoughts?
Any
> Wrapping PassiveExpiringMap in a SynchonizedMap breaks expiration
> -----------------------------------------------------------------
>
> Key: COLLECTIONS-776
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-776
> Project: Commons Collections
> Issue Type: Bug
> Components: Collection
> Affects Versions: 4.4
> Environment: Java 8
> Built on mac OS Catalina 10.15 using gradle 5.2.1 in Intellij
> Reporter: Aidan Sadowski
> Priority: Major
>
> The documentation for PassiveExpiringMap says "If you wish to use this map
> from multiple threads concurrently, you must use appropriate synchronization.
> The simplest approach is to wrap this map using
> [{{Collections.synchronizedMap(Map)}}|https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html?is-external=true#synchronizedMap-java.util.Map-].";
> However, wrapping a PassiveExpiringMap in a Collections.synchronizedMap seems
> to break the PassiveExpiringMap's expiration. Specifically, the operation
> passiveExirpringMap.keySet() no longer removes expired entries prior to
> returning (which it should, according to the PassiveExpiringMap doc).
> I wrote this simple program to prove it. It puts a key into two
> PassiveExpiringMaps with the same expiration policy of 15 seconds, one of
> which is wrapped in a SyncronizedMap. Then it loops through the maps, calling
> keySet() on them. The entry disappears from the unwrapped map after 15
> seconds, but NOT the wrapped map! Code and log output below.
>
> {code:java}
> import java.text.SimpleDateFormat;
> import java.util.Collections;
> import java.util.Date;
> import java.util.Map;
> import java.util.Set;
> import java.util.UUID;
> import java.util.concurrent.TimeUnit;
> import org.apache.commons.collections4.map.PassiveExpiringMap;
> public class Main {
> // unwrapped
> static Map<String, String> unwrappedMap;
> // wrapped
> static Map<String, String> wrappedMap;
> public static void main(String[] args) throws InterruptedException {
> unwrappedMap = new PassiveExpiringMap<>(15, TimeUnit.SECONDS);
> wrappedMap = Collections.synchronizedMap(new PassiveExpiringMap<>(15,
> TimeUnit.SECONDS));
> // Put something in the maps
> String key = UUID.randomUUID().toString();
> logWithTimestamp("Putting key " + key);
> unwrappedMap.put(key, "");
> wrappedMap.put(key, "");
> // Check the map until the key has expired, sleeping 1 second between
> checks
> while (true) {
> Set<String> keysInUnwrappedMap = unwrappedMap.keySet();
> Set<String> keysInWrappedMap = wrappedMap.keySet();
> logWithTimestamp("Keys in unwrapped map: " + String.join(", ",
> keysInUnwrappedMap));
> logWithTimestamp("Keys in wrapped map: " + String.join(", ",
> keysInWrappedMap));
> Thread.sleep(1000);
> }
> }
> private static SimpleDateFormat sdf = new SimpleDateFormat("HH:mm:ss");
> private static void logWithTimestamp(String string) {
> Date resultdate = new Date(System.currentTimeMillis());
> System.out.println("[" + sdf.format(resultdate) + "] " + string);
> }
> }
> {code}
> {code:java}
> [10:02:41] Putting key fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:41] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:41] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:42] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:42] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:43] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:43] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:44] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:44] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:45] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:45] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:46] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:46] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:47] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:47] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:48] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:48] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:49] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:49] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:50] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:50] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:51] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:51] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:52] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:52] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:53] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:53] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:54] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:54] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:55] Keys in unwrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:55] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:56] Keys in unwrapped map:
> [10:02:56] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:57] Keys in unwrapped map:
> [10:02:57] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:58] Keys in unwrapped map:
> [10:02:58] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> [10:02:59] Keys in unwrapped map:
> [10:02:59] Keys in wrapped map: fce9d058-8f13-4e6d-814e-90e03f575678
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
