Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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 @@ -103,17 +103,15 @@ else if (FCGI.Headers.HTTPS.equalsIgnoreCase(name))
processField(field);
}

public void onHeaders()
public Runnable onHeaders()
{
String pathQuery = URIUtil.addPathQuery(_path, _query);
HttpScheme scheme = StringUtil.isEmpty(_secure) ? HttpScheme.HTTP : HttpScheme.HTTPS;
MetaData.Request request = new MetaData.Request(_connection.getBeginNanoTime(), _method, scheme.asString(), hostPort, pathQuery, HttpVersion.fromString(_version), _headers, -1);
Runnable task = _httpChannel.onRequest(request);
_allHeaders.forEach(field -> _httpChannel.getRequest().setAttribute(field.getName(), field.getValue()));
// TODO: here we just execute the task.
// However, we should really return all the way back to onFillable()
// and feed the Runnable to an ExecutionStrategy.
execute(task);
// Return the task to dispatch it after ServerParser.parse() has returned.
return task;
}

private void processField(HttpField field)
Expand Down Expand Up @@ -358,11 +356,6 @@ public boolean onIdleTimeout(TimeoutException timeout)
return !handlingRequest;
}

private void execute(Runnable task)
{
_connection.getConnector().getExecutor().execute(task);
}

private class DemandCallback implements Callback
{
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.nio.ByteBuffer;
import java.util.Set;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeoutException;

import org.eclipse.jetty.fcgi.FCGI;
Expand Down Expand Up @@ -59,6 +60,7 @@ public class ServerFCGIConnection extends AbstractMetaDataConnection implements
private boolean useOutputDirectByteBuffers;
private RetainableByteBuffer inputBuffer;
private HttpStreamOverFCGI stream;
private Runnable onRequest;

public ServerFCGIConnection(Connector connector, EndPoint endPoint, HttpConfiguration configuration, boolean sendStatus200)
{
Expand Down Expand Up @@ -191,7 +193,7 @@ public void onFillable()
{
if (stream == null && inputBuffer.isEmpty())
releaseInputBuffer();
return;
break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why breaking instead of doing the onRequest check and dispatch here? This makes the method slightly less readable.

}
}
else if (read == 0)
Expand All @@ -207,6 +209,24 @@ else if (read == 0)
return;
}
}

// Dispatch only after the parser has returned and input buffer bookkeeping is complete.
Runnable task = onRequest;
onRequest = null;
if (task != null)
{
try
{
getExecutor().execute(task);
}
catch (RejectedExecutionException x)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should never happen as the Jetty connection pool assumes the queue task is infinite. Plus, there's already a catch-all block that is supposed to do the right thing.

{
HttpStreamOverFCGI stream = this.stream;
Runnable failureTask = stream.getHttpChannel().onFailure(x);
this.stream = null;
ThreadPool.executeImmediately(getExecutor(), failureTask);
}
}
}
catch (Exception x)
{
Expand Down Expand Up @@ -359,9 +379,8 @@ public boolean onHeaders(int request)
LOG.debug("Request {} headers on {}", request, stream);
if (stream != null)
{
stream.onHeaders();
// We have dispatched to the application,
// so we must stop the fill & parse loop.
onRequest = stream.onHeaders();
// Return to onFillable() before dispatching to the application.
return true;
}
return false;
Expand Down
Loading
Loading