Permalink
Browse files

Fixed a crash occurs when an application fails to spawn, but the HTTP…

… client disconnects before the error page is generated.

Fixes issue #1028. Calls to clientOutputPipe->write() cause an EPIPE error to be detected,
causing the client to be disconnected. However if there are two clientOutputPipe->write()
calls in succession, then the second one fails with a libeio assertion error because the
client has already been disconnected. The solution is to add more client->connected() checks.
  • Loading branch information...
1 parent 3b7f4af commit c29dca4cfd03a7592bb631c79eb29c621c3ed3a4 @FooBarWidget FooBarWidget committed Jan 28, 2014
Showing with 22 additions and 4 deletions.
  1. +2 −0 NEWS
  2. +4 −0 ext/common/MultiLibeio.cpp
  3. +16 −4 ext/common/agents/HelperAgent/RequestHandler.h
View
2 NEWS
@@ -15,6 +15,8 @@ Release 4.0.37
effect to killing the application process directly with `kill <PID>`,
but killing directly may cause the HTTP client to see an error, while
using this command guarantees that clients see no errors.
+ * Fixed a crash occurs when an application fails to spawn, but the HTTP
+ client disconnects before the error page is generated. Fixes issue #1028.
Release 4.0.36
@@ -55,6 +55,10 @@ struct Data {
: libev(_libev),
callback(_callback)
{
+ // If this assertion fails, then in the context of RequestHandler it means
+ // that it was operating on a client that has already been disconnected.
+ // The RequestHandler code is probably missing some necessary checks on
+ // `client->connected()`.
assert(_libev != NULL);
}
};
@@ -696,8 +696,12 @@ class RequestHandler {
status, (unsigned long) data.size());
client->clientOutputPipe->write(header, pos - header);
- client->clientOutputPipe->write(data.data(), data.size());
- client->clientOutputPipe->end();
+ if (client->connected()) {
+ client->clientOutputPipe->write(data.data(), data.size());
+ if (client->connected()) {
+ client->clientOutputPipe->end();
+ }
+ }
if (client->useUnionStation()) {
snprintf(header, end - header, "Status: %d %s",
@@ -778,8 +782,12 @@ class RequestHandler {
const string header = str.str();
client->clientOutputPipe->write(header.data(), header.size());
- client->clientOutputPipe->write(data.data(), data.size());
- client->clientOutputPipe->end();
+ if (client->connected()) {
+ client->clientOutputPipe->write(data.data(), data.size());
+ if (client->connected()) {
+ client->clientOutputPipe->end();
+ }
+ }
if (client->useUnionStation()) {
client->logMessage("Status: 500 Internal Server Error");
@@ -1094,6 +1102,10 @@ class RequestHandler {
void writeToClientOutputPipe(const ClientPtr &client, const StaticString &data) {
bool wasCommittingToDisk = client->clientOutputPipe->isCommittingToDisk();
bool nowCommittingToDisk = !client->clientOutputPipe->write(data.data(), data.size());
+ if (!client->connected()) {
+ // EPIPE/ECONNRESET detected.
+ return;
+ }
if (!wasCommittingToDisk && nowCommittingToDisk) {
RH_TRACE(client, 3, "Buffering response data to disk; temporarily stopping application socket.");
client->backgroundOperations++;

0 comments on commit c29dca4

Please sign in to comment.