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

Jersey core client 3.1.9 memory leak #5797

Open
jonkjenn opened this issue Nov 12, 2024 · 2 comments · May be fixed by #5794
Open

Jersey core client 3.1.9 memory leak #5797

jonkjenn opened this issue Nov 12, 2024 · 2 comments · May be fixed by #5794

Comments

@jonkjenn
Copy link

If I'm not missing something this is a memory leak if you target unique URLs.

private final ConcurrentHashMap<URL, Lock> locks = new ConcurrentHashMap<>();

@MarioSob
Copy link

This is indeed a memory leak. This map can only increase in size.

I suggest this class to take the following shape:

private static class DefaultConnectionFactory implements ConnectionFactory {
    private final ConcurrentHashMap<URL, Semaphore> locks = new ConcurrentHashMap<>();

    @Override
    public HttpURLConnection getConnection(URL url) throws IOException {
        return connect(url, null);
    }

    @Override
    public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException {
        return connect(url, proxy);
    }

    private HttpURLConnection connect(URL url, Proxy proxy) throws IOException {
        Semaphore thisThreadLock = new Semaphore(0);
        Semaphore previousThreadLock = locks.putIfAbsent(url, thisThreadLock);
        try {
            if (previousThreadLock != null) {
                previousThreadLock.acquireUninterruptibly();
            }
            return proxy == null
                    ? (HttpURLConnection) url.openConnection()
                    : (HttpURLConnection) url.openConnection(proxy);
        } finally {
            locks.remove(url, thisThreadLock);
            thisThreadLock.release();
        }
    }
}

@jansupol
Copy link
Contributor

Thank you for reporting this. We have a fix in progress: #5794

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants