Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TagsCache grows without bounds #191

Closed
lsutic opened this issue Aug 9, 2023 · 2 comments
Closed

TagsCache grows without bounds #191

lsutic opened this issue Aug 9, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@lsutic
Copy link

lsutic commented Aug 9, 2023

Version

4.4.1, 4.4.4

Context

We noticed that the JVM heap was growing for a Vert.x based service and upon doing a heap dump we saw that the TagsCache for four of our event loop contexts had grown to over a million entries.

What follows is my speculation on what happened. I have not been able to reproduce this bug in a test case, but I am working on doing so.

The tag cache size is limited using an implementation of removeEldestEntry in a LinkedHashMap that signals that the entry should be removed if the size of the map exceeds 512. We have seen these maps grow to more than 1M entries.

Concurrent modifications to a LinkedHashMap (which is not allowed) can cause the removeEldestEntry check to fail, as the implementation is not thread safe, and is therefore a way to grow beyond what the removeEldestEntry check would allow.

The TagsCache implementation tries to avoid concurrent access to the cache by doing the following tests:

if (context == null || context.isWorkerContext() || !context.inThread()) {
  // Don't use the cache
}

For an event loop context in a Netty NioEventLoop, the context is non-null, isWorkerContext() returns false, and context.inThread() returns nettyEventLoop().inEventLoop();

I'm not sure this last check (return nettyEventLoop().inEventLoop();) guarantees single-threadedness. For Netty event loops served by multiple threads, it only checks if the calling thread is one of them, not if it is the event loop thread(?).

The result is concurrent access to the cache, which causes it to grow without bounds.

Do you have a reproducer?

Working on it, will update issue when I have one.

Steps to reproduce

(Suspected)

  1. Call TagsCache.getOrCreate concurrently from a context where the context implementation's inThread() returns true for more than one thread.

Extra

Ubuntu 23.04, JDK17

Proof of exceeding LinkedHashMap size:

import java.util.LinkedHashMap;
import java.util.Map;

public class LRUTest {

    public static void main(String[] args) throws Throwable {
        final var map = lruMap();
        var threads = new ArrayList<Thread>();
        for (int t = 0; t < 16; ++t) {
            threads.add(new Thread() {
                public void run() {
                    for (int i = 0; i < 1024000; ++i) {
                        map.put(i, i);
                    }
                }
            });
        }
        for (var t : threads) {
            t.start();
        }
        for (var t : threads) {
            t.join();
        }

        System.out.println(map.size());
    }

    private static LinkedHashMap<Integer,Integer> lruMap() {
        return new LinkedHashMap<Integer,Integer>() {
            @Override
            protected boolean removeEldestEntry(Map.Entry eldest) {
                return size() > 512;
            }
        };
    }
}
@lsutic lsutic added the bug Something isn't working label Aug 9, 2023
@lsutic lsutic closed this as not planned Won't fix, can't repro, duplicate, stale Aug 9, 2023
@lsutic
Copy link
Author

lsutic commented Aug 9, 2023

Got versions mixed up. Ran the 4.4.4 test against 4.4.1

@tsegismont
Copy link
Contributor

Thanks for letting us know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants