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

NullPointerException in BoundedAsyncPool.createIdle() when availableCapacity=0 #1611

Closed
fbotis opened this issue Feb 3, 2021 · 2 comments
Closed
Labels
type: bug A general bug
Milestone

Comments

@fbotis
Copy link

fbotis commented Feb 3, 2021

Bug Report

Looks like there's a null pointer exception if createIdle() is called from different threads and availableCapacity becomes 0 while createIdle() is executed.

Current Behavior

The bug appears in the below method and the scenario is described below:
Thread 1: calls createIdle(): toCreate = 1
Thread 2: calls createIdle(): toCreate =1
Thread 1: reaches the line where makeObject0(future); is called
Thread 2: the if(getAvailableCapacity()) returns 0 thus it breaks . That means the futures array will contain one null element

 @SuppressWarnings("rawtypes")
    CompletableFuture<Void> createIdle() {

        int potentialIdle = getMinIdle() - getIdle();
        if (potentialIdle <= 0 || !isPoolActive()) {
            return CompletableFuture.completedFuture(null);
        }

        int totalLimit = getAvailableCapacity();
        int toCreate = Math.min(Math.max(0, totalLimit), potentialIdle);

        CompletableFuture[] futures = new CompletableFuture[toCreate];
        for (int i = 0; i < toCreate; i++) {

            if (getAvailableCapacity() <= 0) {
                break;
            }

            CompletableFuture<T> future = new CompletableFuture<>();
            futures[i] = future;
            makeObject0(future);

            future.thenAccept(it -> {

                if (isPoolActive()) {
                    idleCount.incrementAndGet();
                    cache.add(it);
                } else {
                    factory.destroy(it);
                }
            });
        }

        return CompletableFuture.allOf(futures);
    }
Stack trace
Caused by: java.lang.NullPointerException: null
        at java.util.concurrent.CompletableFuture.andTree(CompletableFuture.java:1306)
        at java.util.concurrent.CompletableFuture.andTree(CompletableFuture.java:1303)
        at java.util.concurrent.CompletableFuture.allOf(CompletableFuture.java:2225)
        at io.lettuce.core.support.BoundedAsyncPool.createIdle(BoundedAsyncPool.java:184)
        at io.lettuce.core.support.BoundedAsyncPool.acquire0(BoundedAsyncPool.java:236)
        at io.lettuce.core.support.BoundedAsyncPool.acquire(BoundedAsyncPool.java:197)
        at io.lettuce.core.support.AsyncConnectionPoolSupport$1.acquire(AsyncConnectionPoolSupport.java:186)
        at RedisCacheService.setCacheAsync(RedisCacheService.java:232)

Input Code

Input Code

The code where this NPE is thrown is

asyncConnectionPool.acquire()
   .thenCompose(connection -> connection.async().setex(key, ttlSeconds, value)
	.whenComplete((s, throwable) -> {
		asyncConnectionPool.release(connection);
	}));

Expected behavior/code

There shouldn't be any Exception thrown when the available capacity becomes 0

Environment

  • Lettuce version(s): 6.0.2.RELEASE
  • Redis version: 6.0

Possible Solution

The method with the fix (see changes in between <>)

 @SuppressWarnings("rawtypes")
    CompletableFuture<Void> createIdle() {

        int potentialIdle = getMinIdle() - getIdle();
        if (potentialIdle <= 0 || !isPoolActive()) {
            return CompletableFuture.completedFuture(null);
        }

        int totalLimit = getAvailableCapacity();
        int toCreate = Math.min(Math.max(0, totalLimit), potentialIdle);

        **List<CompletableFuture> futures = new ArrayList(toCreate);**
        for (int i = 0; i < toCreate; i++) {

            if (getAvailableCapacity() <= 0) {
                break;
            }

            CompletableFuture<T> future = new CompletableFuture<>();
            **futures.add(future);**
            makeObject0(future);

            future.thenAccept(it -> {

                if (isPoolActive()) {
                    idleCount.incrementAndGet();
                    cache.add(it);
                } else {
                    factory.destroy(it);
                }
            });
        }

        **return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));**
    }

Additional context

@mp911de mp911de added the type: bug A general bug label Feb 3, 2021
mp911de added a commit that referenced this issue Feb 3, 2021
…acity #1611

We now initialize the future array with a completed future instead of leaving the array value null.
mp911de added a commit that referenced this issue Feb 3, 2021
…acity #1611

We now initialize the future array with a completed future instead of leaving the array value null.
@mp911de mp911de added this to the 5.3.7 milestone Feb 3, 2021
mp911de added a commit that referenced this issue Feb 3, 2021
…acity #1611

We now initialize the future array with a completed future instead of leaving the array value null.
@mp911de
Copy link
Collaborator

mp911de commented Feb 3, 2021

Thanks for report, that's fixed now.

@mp911de mp911de closed this as completed Feb 3, 2021
@fbotis
Copy link
Author

fbotis commented Feb 3, 2021

Thanks for the quick fix:)

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

No branches or pull requests

2 participants