How should I use AtomicInteger as a reference counter to manage some resource?

Question

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?

Answer 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.

Answer 2

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.