Skip to content
Open
Changes from all commits
Commits
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
28 changes: 19 additions & 9 deletions eo-runtime/src/main/java/org/eolang/PhCached.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,14 @@
package org.eolang;

import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;

/**
* Cached Phi.
*
* <p>It's highly recommended to use it with {@link PhComposite}.</p>
*
* @since 0.1
* @todo #4884:30min Replace 'synchronized' with ReentrantLock.
* We need to replace 'synchronized' with ReentrantLock to avoid potential
* deadlocks when multiple threads are trying to access the cache simultaneously.
* Moreover, 'synchronized' keyword is forbidden by qulice.
*/
public final class PhCached implements Phi {

Expand All @@ -30,13 +27,19 @@ public final class PhCached implements Phi {
*/
private final AtomicReference<Phi> cached;

/**
* Reentrant lock for thread-safe cache initialization.
*/
private final ReentrantLock lock;

/**
* Ctor.
* @param attr Origin attribute
*/
public PhCached(final Phi attr) {
this.origin = attr;
this.cached = new AtomicReference<>();
this.lock = new ReentrantLock();
}

@Override
Expand All @@ -50,14 +53,21 @@ public boolean hasRho() {
}

@Override
@SuppressWarnings("PMD.AvoidSynchronizedStatement")
public Phi take(final String name) {
synchronized (this.cached) {
if (this.cached.get() == null) {
this.cached.set(this.origin.take(name));
Phi result = this.cached.get();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@AlexNetTie this seems to be thread unsafe code

if (result == null) {
this.lock.lock();
try {
result = this.cached.get();
if (result == null) {
result = this.origin.take(name);
this.cached.set(result);
}
} finally {
this.lock.unlock();
}
}
return this.cached.get();
return result;
}

@Override
Expand Down
Loading