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

GenericItemSourcePool.unlatchAndResetResizing might cause an error #96

Open
ol-loginov opened this issue Aug 5, 2024 · 1 comment
Open

Comments

@ol-loginov
Copy link

Description
I adapt your code to my needs. I have a fork in private repository and mostly trying to use spring-boot-dependcies to manage with jar hell in dependent projects.

So, I've met some tests failing. GenericItemSourcePoolTest.multipleThreadsGetPooledWhenResizePolicyEventuallyCopeWithTheLoad.

After investigation I can report some notes about implementation.
I'd change (and did locally) method GenericItemSourcePool.unlatchAndResetResizing to this variant:

    // you cannot count down latch while resizing is true (in progress). see comments in #awaitResize method 
    private final Consumer<Boolean> unlatchAndResetResizing = result -> {
        var latch = this.countDownLatch.get(); 
        resizing.set(false);
        latch.countDown();
    };

And my comments for awaitResize method:

        // Here some notes about unlatchAndResetResizing. It will fail if another thread is superfast. Look at the following use case:
        // [Thread A] starts resizing (set resizing to true and adds latch, for instance named "Latch1"). The process begins
        // [Thread B] got resizing=true, pass to await - take Latch1 and wait for timeout
        // [Thread A] finish the process and begins "unlatchAndResetResizing" - down Latch1, and ...
        // [Thread B] Latch1 is free! go down to resize again. resizing is still true, so take the latch (latch is still Latch1), awaits it. Latch1 is free! go down to resize again, etc.. And get `(depth > maxRetries)` error
        // [Thread A] ... and we are setting `resizing` to false. At last!
        
        // let's allow only one thread to get in
        if (resizing.compareAndSet(false, true)) {
            countDownLatch.set(new CountDownLatch(1));
            return resize(unlatchAndResetResizing);
        }

Configuration
Just development environment. JDK11 and Maven, no more

Runtime (please complete the following information):

  • Module name and version: current HEAD version of repository
  • Framework/server/module system used: nope
  • JVM: OpenJDK 64-Bit Server VM (build 11.0.24+8-post-Ubuntu-1ubuntu324.04.1, mixed mode, sharing)
  • OS: Ubuntu 24.04 LTS
@rfoltyns
Copy link
Owner

rfoltyns commented Aug 6, 2024

Thank you for reporting this! 🙏

You're right, there is a race there.

I'll try to come up with a fix once I replace this test with a more readable and reliable one.

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

No branches or pull requests

2 participants