Is a non-synchronized WeakHashMap harmful?

Question

I have a code look like this.

private static Map<String, Pattern> PATTERNS;

private static Map<String, Pattern> patterns() {
    if (PATTERNS == null) {
        PATTERNS = new WeakHashMap<>(); // ok? or should be synchronized?
    }
    return PATTERNS;
}

// intending to reuse those pre-compiled patters
private static Pattern pattern(final String regex) {
    return patterns().computeIfAbsent(
            requireNonNull(regex, "regex is null"), Pattern::compile);
}

I already know the WeakHashMap is not synchronized. I just don't care about multiple construction of Patterns.

Should the PATTERNS be synchronized, in case of multi-threaded environment?

Are you trying to implement the [singleton pattern](https://en.wikipedia.org/wiki/Singleton_pattern)? If so, initialize the field when defined, this guarantee thread-safe initialization. If this is too expensive and you need just-in-time thread-safe singleton creation, [this article](https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html) has a working solution at the very end. — Turing85, Jul 14 '19 at 13:23
@Turing85 No, sir. I'm intending to reuse pre-compiled patterns. — Jin Kwon, Jul 14 '19 at 13:27
This sort of pattern cache is best implemented with a LRU map (pretty easily done with a simple `LinkedHashMap` subclass); many applications mostly only use a fairly small number of patterns and you don't want to cache everything forever. _You must still synchronize it._ — Donal Fellows, Feb 03 '23 at 15:06

Tom Hawtin - tackline · Answer 1 · 2019-07-14T13:45:12.580

2

Multi-threaded use of HashMaps can lead to infinite loops. IIRC, concurrent rehashing can leave buckets forming a chain.

Generally, avoid anything with a race condition. There are exceptions, for instance, immutable cache values.

Also:

Types that represent a value, such as String, are not a good type to use as a key to a WeakHashMap.

Lazy initialisation, beyond that which the JVM provides for free, is often not worth it. In this case, it doesn't particularly matter the you may end up with two maps.

edited Jul 14 '19 at 13:45

answered Jul 14 '19 at 13:39

Tom Hawtin - tackline

145,806
30
211
305

@user7294900 I don't have a list of links for everything - you'll have to google it yourself. – Tom Hawtin - tackline Jul 14 '19 at 13:47
Thank you for pointing that, Is it still relevant in Java 8? https://stackoverflow.com/questions/35534906/java-hashmap-getobject-infinite-loop/44180452#44180452 – Ori Marko Jul 14 '19 at 13:59
@user7294900 It's not a bug in the JDK. The implementation has varied so the exact behaviour may change. The general point is that mutable objects used concurrently may cause problems beyond bad outputs. Your code example hints that apparently non-mutable code may have some kind of cache behind the interface, with the same results. – Tom Hawtin - tackline Jul 14 '19 at 15:25

Basil Bourque · Accepted Answer · 2019-07-14T15:33:47.720

2

Is a non-synchronized WeakHashMap harmful?

Yes. You must add additional protection to use WeakHashMap across threads.

Thus the suggestion found in the class Javadoc:

A synchronized WeakHashMap may be constructed using the Collections.synchronizedMap method

PATTERNS = Collections.synchronized( new WeakHashMap<>() ) ;

See this Question.

edited Jul 14 '19 at 15:33

answered Jul 14 '19 at 15:25

Basil Bourque

303,325
100
852
1,154

Is a non-synchronized WeakHashMap harmful?

2 Answers2