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.

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

Fixes #11282 - Deadlocks with DEBUG logging enabled in jetty-server testing. #11306

Merged
merged 1 commit into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -492,13 +492,14 @@ public void setGrowOutput(boolean growOutput)
public String toString()
{
int q;
ByteBuffer b;
Object b;
String o;
try (AutoLock lock = _lock.lock())
try (AutoLock lock = _lock.tryLock())
{
q = _inQ.size();
b = _inQ.peek();
o = BufferUtil.toDetailString(_out);
boolean held = lock.isHeldByCurrentThread();
q = held ? _inQ.size() : -1;
b = held ? _inQ.peek() : "?";
o = held ? BufferUtil.toDetailString(_out) : "?";
}
return String.format("%s[q=%d,q[0]=%s,o=%s]", super.toString(), q, b, o);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,16 +523,18 @@ private boolean lockedLastStreamSend()
@Override
public String toString()
{
try (AutoLock ignored = _lock.lock())
try (AutoLock lock = _lock.tryLock())
{
return String.format("%s@%x{handling=%s, handled=%b, send=%s, completed=%b, request=%s}",
boolean held = lock.isHeldByCurrentThread();
return String.format("%s@%x{handling=%s, handled=%s, send=%s, completed=%s, request=%s}",
this.getClass().getSimpleName(),
hashCode(),
_handling,
_handled,
_streamSendState,
_callbackCompleted,
_request);
held ? _handling : "?",
held ? _handled : "?",
held ? _streamSendState : "?",
held ? _callbackCompleted : "?",
held ? _request : "?"
);
}
}

Expand Down Expand Up @@ -1247,6 +1249,7 @@ public void succeeded()
* <p>
* The implementation maintains the {@link #_streamSendState} before taking
* and serializing the call to the {@link #_writeCallback}, which was set by the call to {@code write}.
*
* @param x The reason for the failure.
*/
@Override
Expand Down Expand Up @@ -1475,6 +1478,7 @@ else if (LOG.isDebugEnabled())

/**
* Called when the {@link Handler} (or it's delegates) fail the request handling.
*
* @param failure The reason for the failure.
*/
@Override
Expand Down Expand Up @@ -1656,6 +1660,7 @@ public void succeeded()

/**
* Called when the error write in {@link HttpChannelState.ChannelCallback#failed(Throwable)} fails.
*
* @param x The reason for the failure.
*/
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

package org.eclipse.jetty.util.thread;

import java.io.Serial;
import java.io.Serializable;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Condition;
Expand All @@ -30,6 +31,7 @@
*/
public class AutoLock implements AutoCloseable, Serializable
{
@Serial
private static final long serialVersionUID = 3300696774541816341L;

private final ReentrantLock _lock = new ReentrantLock();
Expand All @@ -46,8 +48,24 @@ public AutoLock lock()
}

/**
* @see ReentrantLock#isHeldByCurrentThread()
* <p>Tries to acquire the lock.</p>
* <p>Whether the lock was acquired can be tested
* with {@link #isHeldByCurrentThread()}.</p>
* <p>Typical usage of this method is in {@code toString()},
* to avoid deadlocks when the implementation needs to lock
* to retrieve a consistent state to produce the string.</p>
*
* @return this AutoLock for unlocking
*/
Comment on lines +51 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* <p>Tries to acquire the lock.</p>
* <p>Whether the lock was acquired can be tested
* with {@link #isHeldByCurrentThread()}.</p>
* <p>Typical usage of this method is in {@code toString()},
* to avoid deadlocks when the implementation needs to lock
* to retrieve a consistent state to produce the string.</p>
*
* @return this AutoLock for unlocking
*/
* <p>Tries to acquire the lock.</p>
* <p>Whether the lock was acquired can be tested
* with {@link #isHeldByCurrentThread()}.</p>
* <p>Typical usage of this method is in {@code toString()},
* to avoid deadlocks when the implementation needs to lock
* to retrieve a consistent state to produce the string.</p>
* @see ReentrantLock#isHeldByCurrentThread()
* @return this AutoLock for unlocking
*/

public AutoLock tryLock()
{
_lock.tryLock();
return this;
}

/**
* @return whether this lock is held by the current thread
* @see ReentrantLock#isHeldByCurrentThread()
*/
public boolean isHeldByCurrentThread()
{
Expand All @@ -71,7 +89,8 @@ boolean isLocked()
@Override
public void close()
{
_lock.unlock();
if (isHeldByCurrentThread())
_lock.unlock();
}

/**
Expand Down Expand Up @@ -101,6 +120,12 @@ public AutoLock.WithCondition lock()
return (WithCondition)super.lock();
}

@Override
public AutoLock.WithCondition tryLock()
{
return (WithCondition)super.tryLock();
}

/**
* @see Condition#signal()
*/
Expand All @@ -118,20 +143,20 @@ public void signalAll()
}

/**
* @see Condition#await()
* @throws InterruptedException if the current thread is interrupted
* @see Condition#await()
*/
public void await() throws InterruptedException
{
_condition.await();
}

/**
* @see Condition#await(long, TimeUnit)
* @param time the time to wait
* @param unit the time unit
* @return false if the waiting time elapsed
* @throws InterruptedException if the current thread is interrupted
* @see Condition#await(long, TimeUnit)
*/
public boolean await(long time, TimeUnit unit) throws InterruptedException
{
Expand Down