4

I've refined this question a little, removing some of my understanding to try and make it as concise and specific as poss so it might come across as pretty basic

How should I use AtomicInteger as a reference counter to manage clean-up of some resource but in a safe and atomic way?

For instance:

1) Vanilla example:

public void close() {
    if ((refCt.decrementAndGet()) == 0) {
        DBUtil.close(conn);             
    }
}

This is all very well and refCt will be atomic in itself but the value could change in such a way that closing the resource would not be closed - e.g. another thread decrementing the count prior to the conditional.

Use of a var (stack in thread) could ensure refCt is maintained in my thread, I guess but

2) Overkill(?) example:

public void close() {
    synchronized(lock) {
        if ((refCt.decrementAndGet()) == 0) {
            DBUtil.close(conn);             
        }
    }
}

My refCt is atomic in and of itself and my testing of the conditional (external to the AtomicInteger) is synchronised to safeguar atomicity.

3) Should I actually use the AtomicInteger itself to manage the conditional through #compareAndSet and avoid blocking?

public void close() {
    for (;;) {
        int current = refCt.get();
        int refCtDec = current - 1;
        if (compareAndSet(current, refCtDec))
            DBUtil.close(conn);             
    }
}

4) Is this all more complicated than it should be / Can I not see the wood for the trees?

Improve this question

2 Answers 2

Reset to default
3

Unfortunately just an AtomicInteger will not help you, so both 1 and 3 are out. Consider this case:

  1. Thread 1 uses the resource and finishes with it.
  2. Gets into the If statement, sees that no other thread is using it and decrements the counter (atomically) to 0
  3. Scheduler starts thread 2 that increments the counter and starts using resource
  4. Thread 1 wakes up and closes the resource
  5. Thread 2 gets a connection closed for no reason.

An explicit lock is needed to make both changing the counter AND closing a pseudo-atomic operation (i.e. counter may not be changed without acquiring the lock). In that scenario you do not need the Integer to be atomic.

This looks like bad design however - C code written in Java, which never ends well (the other way around is ugly as well). I would consider changing the way resources are shared between the threads.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

Thanks for your response. I understand the implication of refCt hitting 0 and t1 closing a resource then t2 wanting to use it. You're right in that if it happens within the scope of the conditional then I'd be in real trouble. I'm really concerned with: [scenario described in gray's answer's comments].
In the scenario that once threads begin dereferencing no new references can be made - Gray is correct in saying that if all your dereferences are done with decrementAndGet you are fine.
Thanks for your help (and the very worthy point on an increment after the decrement prior to closing).
1

This is all very well and refCt will be atomic in itself but the value could change in such a way that closing the resource would not be closed - e.g. another thread decrementing the count prior to the conditional.

AtomicInteger is (like it's name) is fully atomic. The method decrementAndGet() seems like 2 operations but it is not. Just one thread will see the decremented AtomicInteger as being 0. The only problem would be if you are decrementing it elsewhere.

Another possibility is that threads are incrementing and then decrementing all of the time. So there could be some issues where the DBUtil is closing but then has to open again or something. You may then need a lock but the lock can just be around the DBUtil call and not the AtomicInteger decrement.

2) Overkill(?) example:

Yes I think so. You shouldn't need the lock unless the lock is needed to control access to the DBUtil.

3) Should I actually use the AtomicInteger itself to manage the conditional through #compareAndSet and avoid blocking?

I see no need to do this. Internally the AtomicInteger takes care of the decrementing race conditions. Here's the code for decrementAndGet():

    for (;;) {
        int current = get();
        int next = current - 1;
        if (compareAndSet(current, next))
            return next;
    }

4) Is this all more complicated than it should be / Can I not see the wood for the trees?

See above.

Improve this answer

9 Comments

thanks for your response. The situation is for use with a performance test harness and the situation is, in fact, a lot of threads all referencing and dereferencing some resource. Ignoring re-initialising the resource for now, the question is how to clean up when there are no more references when lots of threads may be dereferencing the resource. I've tackled this story before but with the overkill solution. Can decremented refCt can be relied upon if t1 decrements to 1, t2 decrements to 0, t1 hits "==0" (closing), t2 hits "==0" and tries to close.
referrring to your example : if t1 decrements only via decrementAndGet() then your close() method is thread-safe and there is no problem.
t1 will decrement and 1 will be returned. t2 decrements and 0 will be returned. Only one thread will be == 0. There are no race conditions @wmorrison365 unless you call refCt.get() again.
t1 decrements only through #decrementAndGet
With AtomicInteger, decrementAndGet() is atomic. Only one thread will get 0 @wmorrison365.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.