From 10b06767418f608b99ea027b10bae5646faf513d Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Sat, 12 Jul 2008 03:15:40 +1200 Subject: [PATCH] Author: Christos Tsantilas Bug 2253: Assertion in comm closing sequence (pt 1) --- src/client_side.cc | 14 +++++++----- src/comm.cc | 55 ++++++++++++++++++++++++++++++++-------------- src/disk.cc | 2 +- src/fde.h | 11 +++++++++- 4 files changed, 57 insertions(+), 25 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index 7faf199925..043d9026a7 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -625,7 +625,9 @@ ConnStateData::close() bool ConnStateData::isOpen() const { - return cbdataReferenceValid(this); + return cbdataReferenceValid(this) && // XXX: checking "this" in a method + fd >= 0 && + !fd_table[fd].closing(); } ConnStateData::~ConnStateData() @@ -2301,10 +2303,7 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c connNoteUseOfBuffer(conn, http->req_sz); notedUseOfBuffer = true; - conn->handleRequestBodyData(); - - if (!request->body_pipe->exhausted()) - conn->readSomeData(); + conn->handleRequestBodyData(); // may comm_close and stop producing /* Is it too large? */ @@ -2321,7 +2320,10 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c goto finish; } - context->mayUseConnection(true); + if (!request->body_pipe->productionEnded()) + conn->readSomeData(); + + context->mayUseConnection(!request->body_pipe->productionEnded()); } http->calloutContext = new ClientRequestContext(http); diff --git a/src/comm.cc b/src/comm.cc index 565f879ebd..ae52476cf7 100644 --- a/src/comm.cc +++ b/src/comm.cc @@ -336,7 +336,10 @@ comm_read(int fd, char *buf, int size, AsyncCall::Pointer &callback) { /* Make sure we're not reading anything and we're not closing */ assert(isOpen(fd)); - assert(!fd_table[fd].flags.closing); + assert(!fd_table[fd].flags.closing_); + // XXX: If we already called commio_finish_callback, the new callback + // we are setting here would apply to the next connection with the same FD. + assert(!fd_table[fd].flags.close_request); debugs(5, 4, "comm_read, queueing read for FD " << fd); @@ -1489,6 +1492,23 @@ CommRead::doCallback(comm_err_t errcode, int xerrno) } } +void +comm_close_start(int fd, void *data) +{ + fde *F = &fd_table[fd]; + + F->flags.closing_ = 1; + +#if USE_SSL + + if (F->ssl) + ssl_shutdown_method(fd); + +#endif + +} + + void comm_close_complete(int fd, void *data) { @@ -1541,7 +1561,10 @@ _comm_close(int fd, char const *file, int line) fdd_table[fd].close_file = file; fdd_table[fd].close_line = line; - if (F->flags.closing) + if(F->flags.close_request) + return; + + if (F->flags.closing_) return; if (shutting_down && (!F->flags.open || F->type == FD_FILE)) @@ -1556,14 +1579,14 @@ _comm_close(int fd, char const *file, int line) PROF_start(comm_close); - F->flags.closing = 1; - -#if USE_SSL - - if (F->ssl) - ssl_shutdown_method(fd); + F->flags.close_request = 1; -#endif + AsyncCall::Pointer startCall=commCbCall(5,4, "comm_close_start", + CommCloseCbPtrFun(comm_close_start, NULL)); + typedef CommCloseCbParams Params; + Params &startParams = GetCommParams(startCall); + startParams.fd = fd; + ScheduleCallHere(startCall); commSetTimeout(fd, -1, NULL, NULL); @@ -1586,12 +1609,11 @@ _comm_close(int fd, char const *file, int line) comm_empty_os_read_buffers(fd); - AsyncCall::Pointer call=commCbCall(5,4, "comm_close_complete", + AsyncCall::Pointer completeCall=commCbCall(5,4, "comm_close_complete", CommCloseCbPtrFun(comm_close_complete, NULL)); - typedef CommCloseCbParams Params; - Params ¶ms = GetCommParams(call); - params.fd = fd; - ScheduleCallHere(call); + Params &completeParams = GetCommParams(completeCall); + completeParams.fd = fd; + ScheduleCallHere(completeCall); PROF_stop(comm_close); } @@ -1995,7 +2017,7 @@ comm_write(int fd, const char *buf, int size, IOCB * handler, void *handler_data void comm_write(int fd, const char *buf, int size, AsyncCall::Pointer &callback, FREE * free_func) { - assert(!fd_table[fd].flags.closing); + assert(!fd_table[fd].flags.closing_); debugs(5, 5, "comm_write: FD " << fd << ": sz " << size << ": asynCall " << callback << "."); @@ -2167,7 +2189,6 @@ comm_listen(int sock) { return sock; } -// AcceptFD::callback() wrapper void comm_accept(int fd, IOACB *handler, void *handler_data) { debugs(5, 5, "comm_accept: FD " << fd << " handler: " << (void*)handler); @@ -2234,7 +2255,7 @@ AcceptFD::acceptOne() { if (newfd == COMM_NOMESSAGE) { /* register interest again */ - debugs(5, 5, "AcceptFD::acceptOne eof: FD " << fd << + debugs(5, 5, HERE << "try later: FD " << fd << " handler: " << *theCallback); commSetSelect(fd, COMM_SELECT_READ, comm_accept_try, NULL, 0); return false; diff --git a/src/disk.cc b/src/disk.cc index 69897e1a46..b78afe37e9 100644 --- a/src/disk.cc +++ b/src/disk.cc @@ -133,7 +133,7 @@ file_close(int fd) */ assert(F->write_handler == NULL); - F->flags.closing = 1; + F->flags.closing_ = 1; #if CALL_FSYNC_BEFORE_CLOSE diff --git a/src/fde.h b/src/fde.h index 517c54dfce..a4403af0ad 100644 --- a/src/fde.h +++ b/src/fde.h @@ -49,6 +49,15 @@ public: memset(this, 0, sizeof(fde)); local_addr.SetEmpty(); // IPAddress likes to be setup nicely. } + + /** + * Return true if the the comm_close for this fd called. + * Two flags used for the filedescriptor closing procedure: + * - The flag flags.close_request which set when the comm_close called + * - The flag flags.closing which scheduled to be set just before the + * comm_close handlers called. + */ + bool closing() {return flags.closing_ || flags.close_request;} /* NOTE: memset is used on fdes today. 20030715 RBC */ static void DumpStats (StoreEntry *); @@ -72,7 +81,7 @@ public: unsigned int open:1; unsigned int close_request:1; unsigned int write_daemon:1; - unsigned int closing:1; + unsigned int closing_:1; unsigned int socket_eof:1; unsigned int nolinger:1; unsigned int nonblocking:1; -- 2.47.2