Merge pull request #2528 from eclipse/jetty-9.4.x-2525-SharedBlockingCallback

Issue #2525 Clean up SharedBlockingCallback
This commit is contained in:
Simone Bordet 2018-05-15 11:00:28 +02:00 committed by GitHub
commit faaa64c5b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 68 additions and 32 deletions

View File

@ -62,7 +62,7 @@
<Set name="headerCacheSize"><Property name="jetty.httpConfig.headerCacheSize" default="512" /></Set>
<Set name="delayDispatchUntilContent"><Property name="jetty.httpConfig.delayDispatchUntilContent" deprecated="jetty.delayDispatchUntilContent" default="true"/></Set>
<Set name="maxErrorDispatches"><Property name="jetty.httpConfig.maxErrorDispatches" default="10"/></Set>
<Set name="blockingTimeout"><Property name="jetty.httpConfig.blockingTimeout" default="-1"/></Set>
<Set name="blockingTimeout"><Property deprecated="jetty.httpConfig.blockingTimeout" default="-1"/></Set>
<Set name="persistentConnectionsEnabled"><Property name="jetty.httpConfig.persistentConnectionsEnabled" default="true"/></Set>
<Set name="cookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.cookieCompliance" default="RFC6265"/></Arg></Call></Set>
<Set name="multiPartFormDataCompliance"><Call class="org.eclipse.jetty.server.MultiPartFormDataCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.multiPartFormDataCompliance" default="RFC7578"/></Arg></Call></Set>

View File

@ -59,9 +59,6 @@ etc/jetty.xml
## Maximum number of error dispatches to prevent looping
# jetty.httpConfig.maxErrorDispatches=10
## Maximum time to block in total for a blocking IO operation (default -1 is to use idleTimeout on progress)
# jetty.httpConfig.blockingTimeout=-1
## Cookie compliance mode of: RFC2965, RFC6265
# jetty.httpConfig.cookieCompliance=RFC6265

View File

@ -31,6 +31,8 @@ import org.eclipse.jetty.util.TreeTrie;
import org.eclipse.jetty.util.Trie;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
/**
* HTTP Configuration.
@ -46,6 +48,8 @@ import org.eclipse.jetty.util.annotation.ManagedObject;
@ManagedObject("HTTP Configuration")
public class HttpConfiguration
{
private static final Logger LOG = Log.getLogger(HttpConfiguration.class);
public static final String SERVER_VERSION = "Jetty(" + Jetty.VERSION + ")";
private final List<Customizer> _customizers=new CopyOnWriteArrayList<>();
private final Trie<Boolean> _formEncodedMethods = new TreeTrie<>();
@ -239,8 +243,10 @@ public class HttpConfiguration
*
* @return -1, for no blocking timeout (default), 0 for a blocking timeout equal to the
* idle timeout; &gt;0 for a timeout in ms applied to the total blocking operation.
* @deprecated Replaced by {@link #getMinResponseDataRate()} and {@link #getMinRequestDataRate()}
*/
@ManagedAttribute("Total timeout in ms for blocking I/O operations.")
@ManagedAttribute(value = "Total timeout in ms for blocking I/O operations. DEPRECATED!", readonly = true)
@Deprecated
public long getBlockingTimeout()
{
return _blockingTimeout;
@ -254,9 +260,13 @@ public class HttpConfiguration
*
* @param blockingTimeout -1, for no blocking timeout (default), 0 for a blocking timeout equal to the
* idle timeout; &gt;0 for a timeout in ms applied to the total blocking operation.
* @deprecated Replaced by {@link #setMinResponseDataRate(long)} and {@link #setMinRequestDataRate(long)}
*/
@Deprecated
public void setBlockingTimeout(long blockingTimeout)
{
if (blockingTimeout>0)
LOG.warn("HttpConfiguration.setBlockingTimeout is deprecated!");
_blockingTimeout = blockingTimeout;
}

View File

@ -48,7 +48,7 @@ import org.eclipse.jetty.util.log.Logger;
*/
public class SharedBlockingCallback
{
static final Logger LOG = Log.getLogger(SharedBlockingCallback.class);
private static final Logger LOG = Log.getLogger(SharedBlockingCallback.class);
private static Throwable IDLE = new ConstantThrowable("IDLE");
private static Throwable SUCCEEDED = new ConstantThrowable("SUCCEEDED");
@ -60,6 +60,7 @@ public class SharedBlockingCallback
private final Condition _complete = _lock.newCondition();
private Blocker _blocker = new Blocker();
@Deprecated
protected long getIdleTimeout()
{
return -1;
@ -135,7 +136,11 @@ public class SharedBlockingCallback
_complete.signalAll();
}
else
throw new IllegalStateException(_state);
{
LOG.warn("Succeeded after {}",_state.toString());
if (LOG.isDebugEnabled())
LOG.debug(_state);
}
}
finally
{
@ -167,8 +172,12 @@ public class SharedBlockingCallback
}
else
{
cause.printStackTrace(System.err);
throw new IllegalStateException(_state);
LOG.warn("Failed {} in {}",cause.toString(),_state.toString());
if (LOG.isDebugEnabled())
{
LOG.debug(cause);
LOG.debug(_state);
}
}
}
finally
@ -227,6 +236,7 @@ public class SharedBlockingCallback
}
catch (final InterruptedException e)
{
_state = e;
throw new InterruptedIOException();
}
finally
@ -253,9 +263,9 @@ public class SharedBlockingCallback
{
try
{
// If the blocker timed itself out, remember the state
if (_state instanceof BlockerTimeoutException)
// and create a new Blocker
// If we have a failure
if (_state!=null && _state!=SUCCEEDED)
// create a new Blocker
_blocker=new Blocker();
else
// else reuse Blocker

View File

@ -18,25 +18,30 @@
package org.eclipse.jetty.util;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jetty.util.SharedBlockingCallback.Blocker;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.hamcrest.Matchers;
import org.junit.Assert;
import org.junit.Test;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
public class SharedBlockingCallbackTest
{
private static final Logger LOG = Log.getLogger(SharedBlockingCallback.class);
final AtomicInteger notComplete = new AtomicInteger();
final SharedBlockingCallback sbcb= new SharedBlockingCallback()
{
@ -52,14 +57,8 @@ public class SharedBlockingCallbackTest
super.notComplete(blocker);
notComplete.incrementAndGet();
}
};
public SharedBlockingCallbackTest()
{
}
@Test
public void testDone() throws Exception
{
@ -211,7 +210,7 @@ public class SharedBlockingCallbackTest
{
try (Blocker blocker=sbcb.acquire())
{
SharedBlockingCallback.LOG.info("Blocker not complete "+blocker+" warning is expected...");
LOG.info("Blocker not complete "+blocker+" warning is expected...");
}
Assert.assertEquals(1,notComplete.get());
@ -220,6 +219,7 @@ public class SharedBlockingCallbackTest
@Test
public void testBlockerTimeout() throws Exception
{
LOG.info("Succeeded after ... warning is expected...");
Blocker b0=null;
try
{
@ -238,21 +238,40 @@ public class SharedBlockingCallbackTest
}
Assert.assertEquals(0,notComplete.get());
try (Blocker blocker=sbcb.acquire())
{
assertThat(blocker,not(equalTo(b0)));
assertThat(blocker,not(sameInstance(b0)));
b0.succeeded();
blocker.succeeded();
}
}
@Test
public void testInterruptedException() throws Exception
{
Blocker blocker0;
try (Blocker blocker = sbcb.acquire())
{
blocker0 = blocker;
Thread.currentThread().interrupt();
try
{
b0.succeeded();
blocker.block();
fail();
}
catch(Exception e)
catch (InterruptedIOException ignored)
{
assertThat(e,instanceOf(IllegalStateException.class));
assertThat(e.getCause(),instanceOf(TimeoutException.class));
}
}
// Blocker.close() has been called by try-with-resources.
// Simulate callback completion, must not throw.
LOG.info("Succeeded after ... warning is expected...");
blocker0.succeeded();
try (Blocker blocker = sbcb.acquire())
{
assertThat(blocker, not(sameInstance(blocker0)));
blocker.succeeded();
}
}