Skip to content

Commit

Permalink
Revert "TS-1007: SSLN Close called before TXN Close. This closes #249."
Browse files Browse the repository at this point in the history
This reverts commit 2e06897.

This breaks in multiple ways, see TS-3804 and TS-3805. Sudheer was able to fix
one, but we stopped after a few unsuccessful attempts on the second. We should
move out TS-1007, TS-3804 and TS-3805 until later.
  • Loading branch information
zwoop committed Jul 31, 2015
1 parent 35f11dd commit 1d1eaa6
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 38 deletions.
5 changes: 1 addition & 4 deletions iocore/net/UnixNet.cc
Expand Up @@ -672,10 +672,7 @@ NetHandler::_close_vc(UnixNetVConnection *vc, ink_hrtime now, int &handle_event,
++closed;
} else {
vc->next_inactivity_timeout_at = now;
// Found a case where the keep_alive_queue.head was NULL while running regression
// with non-standard configuration
if (keep_alive_queue.head)
keep_alive_queue.head->handleEvent(EVENT_IMMEDIATE, NULL);
keep_alive_queue.head->handleEvent(EVENT_IMMEDIATE, NULL);
++handle_event;
}
}
Expand Down
8 changes: 0 additions & 8 deletions proxy/http/HttpClientSession.cc
Expand Up @@ -92,11 +92,6 @@ HttpClientSession::destroy()
HTTP_DECREMENT_DYN_STAT(http_current_client_connections_stat);
conn_decrease = false;
}
// Make sure we clean up ua_session in our HttpSM
// Otherwise, we will try to double free it in HttpSM::kill_this
if (current_reader) {
current_reader->ua_session = NULL;
}

super::destroy();
THREAD_FREE(this, httpClientSessionAllocator, this_thread());
Expand Down Expand Up @@ -332,9 +327,6 @@ HttpClientSession::do_io_close(int alerrno)
HTTP_SUM_DYN_STAT(http_transactions_per_client_con, transact_count);
HTTP_DECREMENT_DYN_STAT(http_current_client_connections_stat);
conn_decrease = false;

// Should only be triggering the hooks for session close
// if this is a SSLNetVConnection or a UnixNetVConnection
do_api_callout(TS_HTTP_SSN_CLOSE_HOOK);
}
}
Expand Down
30 changes: 8 additions & 22 deletions proxy/http/HttpSM.cc
Expand Up @@ -3189,10 +3189,13 @@ HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c)
ua_session->set_half_close_flag();
}

// TS-1007, delaying ua_session->do_io_close to kill_this
// so the session_close hook occurs after the transaction close hook
// Also delaying the session release to kill_this in the keep_alive case
// so we don't lose any keep-alive opportunities
ua_session->do_io_close();
ua_session = NULL;
} else {
ink_assert(ua_buffer_reader != NULL);
ua_session->release(ua_buffer_reader);
ua_buffer_reader = NULL;
ua_session = NULL;
}

return 0;
Expand Down Expand Up @@ -6542,6 +6545,7 @@ HttpSM::kill_this()
plugin_tunnel = NULL;
}

ua_session = NULL;
server_session = NULL;

// So we don't try to nuke the state machine
Expand All @@ -6566,24 +6570,6 @@ HttpSM::kill_this()
reentrancy_count--;
ink_release_assert(reentrancy_count == 0);

// Delay the close of the user agent session, so the close session
// occurs after the close transaction
if (ua_session) {
// If this is a keep-alive client connection, just relase the client
// session rather than closing it.
if (t_state.client_info.keep_alive == HTTP_KEEPALIVE &&
(t_state.www_auth_content != HttpTransact::CACHE_AUTH_SERVE || ua_session->get_bound_ss())) {
// successful keep-alive, release the client session instead of destroying it
ink_assert(ua_buffer_reader != NULL);
ua_session->release(ua_buffer_reader);
ua_buffer_reader = NULL;
} else {
// Not keep alive, go ahead and shut it down
ua_session->do_io_close();
}
ua_session = NULL;
}

// If the api shutdown & list removeal was synchronous
// then the value of kill_this_async_done has changed so
// we must check it again
Expand Down
8 changes: 5 additions & 3 deletions proxy/spdy/SpdyClientSession.cc
Expand Up @@ -114,6 +114,7 @@ SpdyClientSession::init(NetVConnection *netvc)

this->vc->set_inactivity_timeout(HRTIME_SECONDS(spdy_accept_no_activity_timeout));
vc->add_to_keep_alive_queue();
SET_HANDLER(&SpdyClientSession::state_session_start);
}

void
Expand Down Expand Up @@ -197,11 +198,11 @@ SpdyClientSession::new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOBu
sm->resp_buffer = TSIOBufferCreate();
sm->resp_reader = TSIOBufferReaderAlloc(sm->resp_buffer);

do_api_callout(TS_HTTP_SSN_START_HOOK);
eventProcessor.schedule_imm(sm, ET_NET);
}

void
SpdyClientSession::start()
int
SpdyClientSession::state_session_start(int /* event */, void * /* edata */)
{
const spdylay_settings_entry entries[] = {
{SPDYLAY_SETTINGS_MAX_CONCURRENT_STREAMS, SPDYLAY_ID_FLAG_SETTINGS_NONE, spdy_max_concurrent_streams},
Expand All @@ -228,6 +229,7 @@ SpdyClientSession::start()
}

TSVIOReenable(this->write_vio);
return EVENT_CONT;
}

int
Expand Down
7 changes: 6 additions & 1 deletion proxy/spdy/SpdyClientSession.h
Expand Up @@ -117,7 +117,12 @@ class SpdyClientSession : public ProxyClientSession, public PluginIdentity
ink_release_assert(false);
return NULL;
}
void start();

void
start()
{
ink_release_assert(false);
}

void do_io_close(int lerrno = -1);
void
Expand Down

0 comments on commit 1d1eaa6

Please sign in to comment.