Revert "Revert "Fixed AsyncIO double dispatch""

This reverts commit 8bd94ec6be.
This is a revert of the revert to add back in the useful debug, plus some TODO comments describing the problems
This commit is contained in:
Greg Wilkins 2015-02-20 00:18:29 +11:00
parent 8bd94ec6be
commit b60ea47ef4
7 changed files with 24 additions and 5 deletions

View File

@ -21,6 +21,8 @@ package org.eclipse.jetty.http2.server;
import org.eclipse.jetty.server.HttpChannelState;
import org.eclipse.jetty.server.HttpInput;
// TODO This class is the same as the default. Is it needed?
public class HttpInputOverHTTP2 extends HttpInput
{
private final HttpChannelState _httpChannelState;

View File

@ -60,6 +60,8 @@ public abstract class FillInterest
}
try
{
if (LOG.isDebugEnabled())
LOG.debug("{} register {}",this,callback);
needsFillInterest();
}
catch (Throwable e)
@ -78,7 +80,7 @@ public abstract class FillInterest
LOG.debug("{} fillable {}",this,callback);
if (callback != null && _interested.compareAndSet(callback, null))
callback.succeeded();
else
else if (LOG.isDebugEnabled())
LOG.debug("{} lost race {}",this,callback);
}

View File

@ -102,11 +102,16 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
protected HttpInput newHttpInput()
{
// TODO - this is a convoluted way to construct the HttpInput. Can this be simplified?
// TODO - the issue is that the HttpInput needs access to the HttpState, which is not
// TODO - constructed until the HttpChannel is constructed... so there is chicken and egg.
return new HttpInput()
{
@Override
protected void onReadPossible()
{
// TODO All three implementations just do this? do we need to override onReadPossible at all?
getState().onReadPossible();
}
};

View File

@ -730,8 +730,8 @@ public class HttpChannelState
}
}
// TODO, do we need to execute? or should it be done elsewhere?
if (handle)
// TODO, do we need to execute or just run?
_channel.execute(_channel);
}

View File

@ -538,7 +538,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
{
_input.failed(x);
}
}
private class AsyncReadCallback implements Callback
@ -547,8 +546,13 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
public void succeeded()
{
if (parseContent())
_channel.handle();
_channel.handle(); // TODO this call to handle can be duplicated by HttpInput.addContent calling onReadPossible
else if (!_input.isFinished())
// TODO This may not always be correct. The main use-case is when the asyncReadCallback has succeeded because of
// some data that is not sufficient to produce anything to read (Eg one byte of a chunk header). We can't add
// zero length content because HttpInput.read() cannot return 0 as no bytes read! So instead we just say we are
// fill interested and look for more content. BUT maybe there is a case when this is not needed..... hmmm I think
// this is probably OK as the AsyncReadCallback is only ever used when there is not another thread reading etc.
asyncReadFillInterested();
}
@ -560,7 +564,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
}
}
private class SendCallback extends IteratingCallback
{
private MetaData.Response _info;

View File

@ -347,6 +347,7 @@ public abstract class HttpInput extends ServletInputStream implements Runnable
}
}
// TODO currently this is an active method that can dispatch a call to HttpChannel.handle. This can be duplicated by the AsyncReadCallback!
if (call_on_read_possible)
onReadPossible();
}

View File

@ -356,6 +356,8 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo
@Override
public void execute(Runnable job)
{
if (LOG.isDebugEnabled())
LOG.debug("queue {}",job);
if (!isRunning() || !_jobs.offer(job))
{
LOG.warn("{} rejected {}", this, job);
@ -552,7 +554,11 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo
// Job loop
while (job != null && isRunning())
{
if (LOG.isDebugEnabled())
LOG.debug("run {}",job);
runJob(job);
if (LOG.isDebugEnabled())
LOG.debug("ran {}",job);
if (Thread.interrupted())
{
ignore=true;