HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

java.util.concurrent bounded resuable resource implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
utilresourceresuablejavaboundedimplementationconcurrent

Problem

I'm trying to code following requirements with lockfree in most-used-code-path that is to get a resource from pool.

-
Same resource should be used n (maxUsageCount) number of times and then
destroyed and create a new one.

-
If there is an error with a resource, it should be marked stale by
client. Staled resource should be destroyed and not handed out to
the clients, a new resource should be created.

-
Resource must be closed after maxUsageCount otherwise its leak.

Resource is threadsafe.

```
public class ReusableResource {

public static interface Factory {
T create();
}

private final int maxUsageCount;
private final AtomicReference> resources;
private final Object lock;
private final ReusableResource.Factory factory;

public ReusableResource(int maxUsageCount,
ReusableResource.Factory factory) {
if (maxUsageCount >(
new ConcurrentLinkedQueue());
lock = new Object();
this.factory = factory;
}

public T get() {
final ConcurrentLinkedQueue tmpq = resources.get(); // load once

T res = tmpq.poll();

if (res == null) {
synchronized (lock) {
res = tmpq.poll(); // double check
// empty queue, first time or none left
if (res == null) {
final ConcurrentLinkedQueue newq = new ConcurrentLinkedQueue(
Collections.nCopies(maxUsageCount, factory.create()));
// checkout the one we are returning
res = newq.poll();
// overwrite q object, do not modify existing
this.resources.set(newq);
// no stale check on newly created instance
return res; // newq = new ConcurrentLinkedQueue(
Collections.nCopies(maxUsageCount, factory.create()));
// checkout the one we are returning
r

Solution

Welcome to Code Review.

You have posted an very good quality post and that's why it takes so long to get an answer.

I have 2 minor remarks to make.

First :

if (maxUsageCount < 0) throw new IllegalArgumentException();


You do this check 3 times, I have no problem with that.

But what if your maxUsageCount is 0 ?

I think this is an unwanted situation so better to do :

if (maxUsageCount <= 0) throw new IllegalArgumentException();


Second :

The clientCode :

QueueConnection qc = null;
try {
     qc = reuse.get();
} finally {
    try { qc.close(); }
    catch (JMSException e) { e.printStackTrace(); }
}


You do not check if qc is null, what is possible when you get an exception.
You have 2 options for this.

QueueConnection qc = null;
try {
     qc = reuse.get();
} finally {
    if (qc!=null) {
        try { qc.close(); }
        catch (JMSException e) { e.printStackTrace(); }
    }
}


edit :

This last one is wrong cause QueueConnection doesn't implement AutoClosable.

Or from java 7 you can use try-with-resource
When you extend QueueConnection and implement the AutoClosable you can do the following :

try (QueueConnectionAutoClosable qc = reuse.get()) {
    //custom code what you need to do.
}

Code Snippets

if (maxUsageCount < 0) throw new IllegalArgumentException();
if (maxUsageCount <= 0) throw new IllegalArgumentException();
QueueConnection qc = null;
try {
     qc = reuse.get();
} finally {
    try { qc.close(); }
    catch (JMSException e) { e.printStackTrace(); }
}
QueueConnection qc = null;
try {
     qc = reuse.get();
} finally {
    if (qc!=null) {
        try { qc.close(); }
        catch (JMSException e) { e.printStackTrace(); }
    }
}
try (QueueConnectionAutoClosable qc = reuse.get()) {
    //custom code what you need to do.
}

Context

StackExchange Code Review Q#47816, answer score: 5

Revisions (0)

No revisions yet.