patternjavaMinor
java.util.concurrent bounded resuable resource implementation
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 (
destroyed and create a new one.
-
If there is an error with a resource, it should be marked
client. Staled resource should be destroyed and not handed out to
the clients, a new resource should be created.
-
Resource must be closed after
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
-
Same resource should be used n (
maxUsageCount) number of times and thendestroyed and create a new one.
-
If there is an error with a resource, it should be marked
stale byclient. 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 :
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 :
Second :
The clientCode :
You do not check if qc is null, what is possible when you get an exception.
You have 2 options for this.
edit :
This last one is wrong cause
Or from java 7 you can use try-with-resource
When you extend
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.