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.

Sign up for GitHub

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

Support for loom #230

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Support for loom #230

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
65b863d
Support for loom
zenghu1chen Jul 14, 2023
f07478a
add import
zenghu1chen Jul 14, 2023
dcc2217
Loom Support
zenghu1chen Jul 17, 2023
9b989b5
Merge remote-tracking branch 'upstream/master'
zenghu1chen Feb 27, 2024
9e9dc74
Loom Support
zenghu1chen Feb 27, 2024
43aed56
remove trailing spaces
zenghu1chen Feb 27, 2024
887d8a9
lock before try block
zenghu1chen Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/java/org/apache/commons/pool2/PooledObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.time.Duration;
import java.time.Instant;
import java.util.Deque;
import java.util.concurrent.locks.ReentrantLock;

/**
* Defines the wrapper that is used to track the additional information, such as
Expand All @@ -33,6 +34,8 @@
*/
public interface PooledObject<T> extends Comparable<PooledObject<T>> {

ReentrantLock lock = new ReentrantLock();
Copy link
Member

@garydgregory garydgregory Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A global variable? That looks like an anti-pattern. -1 unless you can explain what I am misunderstanding.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lock is to replace synchronized(PooledObject) with ReentrantLock in the GenericObjectPool,BaseGenericObjectPool and GenericKeyedObjectPool class.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @zenghu1chen
A mutable global variable in an interface is an anti-pattern IMO, you've not addressed this issue in your comment.


/**
* Tests whether the given PooledObject is null <em>or</em> contains a null.
*
Expand Down
8 changes: 6 additions & 2 deletions src/main/java/org/apache/commons/pool2/impl/BaseGenericObjectPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Collectors;

import javax.management.InstanceAlreadyExistsException;
Expand Down Expand Up @@ -358,7 +359,7 @@ public String toString() {
final Object closeLock = new Object();
volatile boolean closed;

final Object evictionLock = new Object();
final ReentrantLock evictionLock = new ReentrantLock ();
private Evictor evictor; // @GuardedBy("evictionLock")
EvictionIterator evictionIterator; // @GuardedBy("evictionLock")

Expand Down Expand Up @@ -1808,7 +1809,8 @@ public final void setTimeBetweenEvictionRunsMillis(final long timeBetweenEvictio
* @param delay time in milliseconds before start and between eviction runs
*/
final void startEvictor(final Duration delay) {
synchronized (evictionLock) {
try {
evictionLock.lock();
final boolean isPositiverDelay = PoolImplUtils.isPositive(delay);
if (evictor == null) { // Starting evictor for the first time or after a cancel
if (isPositiverDelay) { // Starting new evictor
Expand All @@ -1826,6 +1828,8 @@ final void startEvictor(final Duration delay) {
} else { // Stopping evictor
EvictionTimer.cancel(evictor, evictorShutdownTimeoutDuration, false);
}
} finally {
evictionLock.unlock();
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,8 @@ public void evict() throws E {
PooledObject<T> underTest = null;
final EvictionPolicy<T> evictionPolicy = getEvictionPolicy();

synchronized (evictionLock) {
try {
evictionLock.lock();
final EvictionConfig evictionConfig = new EvictionConfig(
getMinEvictableIdleDuration(),
getSoftMinEvictableIdleDuration(),
Expand Down Expand Up @@ -1106,6 +1107,8 @@ public void evict() throws E {
// states are used
}
}
} finally {
evictionLock.unlock();
}
}
final AbandonedConfig ac = this.abandonedConfig;
Expand Down
10 changes: 8 additions & 2 deletions src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,8 @@ public void evict() throws E {
PooledObject<T> underTest = null;
final EvictionPolicy<T> evictionPolicy = getEvictionPolicy();

synchronized (evictionLock) {
try {
evictionLock.lock();
final EvictionConfig evictionConfig = new EvictionConfig(
getMinEvictableIdleDuration(),
getSoftMinEvictableIdleDuration(),
Expand Down Expand Up @@ -785,6 +786,8 @@ public void evict() throws E {
// states are used
}
}
} finally {
evictionLock.unlock();
}
}
final AbandonedConfig ac = this.abandonedConfig;
Expand Down Expand Up @@ -956,10 +959,13 @@ public void invalidateObject(final T obj, final DestroyMode destroyMode) throws
throw new IllegalStateException(
"Invalidated object not currently part of this pool");
}
synchronized (p) {
try {
p.lock.lock();
if (p.getState() != PooledObjectState.INVALID) {
destroy(p, destroyMode);
}
} finally {
p.lock.unlock();
}
ensureIdle(1, false);
}
Expand Down