Skip to content

Fix lost pending_actions causing actions on stale objects. #5235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bryancall merged 1 commit into from
Apr 11, 2019
Merged

Fix lost pending_actions causing actions on stale objects. #5235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions iocore/net/P_UnixNetVConnection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ class UnixNetVConnection : public NetVConnection
void cancel_active_timeout() override;
void cancel_inactivity_timeout() override;
void set_action(Continuation *c) override;
const Action *get_action() const;
void add_to_keep_alive_queue() override;
void remove_from_keep_alive_queue() override;
bool add_to_active_queue() override;
Expand Down Expand Up @@ -403,6 +404,12 @@ UnixNetVConnection::set_action(Continuation *c)
action_ = c;
}

TS_INLINE const Action *
UnixNetVConnection::get_action() const
{
return &action_;
}

// declarations for local use (within the net module)

void write_to_net(NetHandler *nh, UnixNetVConnection *vc, EThread *thread);
Expand Down
29 changes: 22 additions & 7 deletions proxy/http/HttpSM.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1725,14 +1725,16 @@ HttpSM::state_http_server_open(int event, void *data)
{
SMDebug("http_track", "entered inside state_http_server_open");
STATE_ENTER(&HttpSM::state_http_server_open, event);
// TODO decide whether to uncomment after finish testing redirect
// ink_assert(server_entry == NULL);
pending_action = nullptr;
ink_release_assert(event == EVENT_INTERVAL || event == NET_EVENT_OPEN || event == NET_EVENT_OPEN_FAILED ||
pending_action == nullptr);
if (event != NET_EVENT_OPEN) {
pending_action = nullptr;
}
milestones[TS_MILESTONE_SERVER_CONNECT_END] = Thread::get_hrtime();
HttpServerSession *session;

switch (event) {
case NET_EVENT_OPEN:
case NET_EVENT_OPEN: {
session = (TS_SERVER_SESSION_SHARING_POOL_THREAD == t_state.http_config_param->server_session_sharing_pool) ?
THREAD_ALLOC_INIT(httpServerSessionAllocator, mutex->thread_holding) :
httpServerSessionAllocator.alloc();
Expand All @@ -1752,7 +1754,12 @@ HttpSM::state_http_server_open(int event, void *data)
printf("client fd is :%d , server fd is %d\n",vc->con.fd,
server_vc->con.fd); */
session->attach_hostname(t_state.current.server->name);
session->new_connection(static_cast<NetVConnection *>(data));
UnixNetVConnection *vc = static_cast<UnixNetVConnection *>(data);
ink_release_assert(pending_action == nullptr || pending_action == vc->get_action());
pending_action = nullptr;

session->new_connection(vc);

session->state = HSS_ACTIVE;

attach_server_session(session);
Expand All @@ -1766,6 +1773,7 @@ HttpSM::state_http_server_open(int event, void *data)
}
handle_http_server_open();
return 0;
}
case EVENT_INTERVAL: // Delayed call from another thread
if (server_session == nullptr) {
do_http_server_open();
Expand Down Expand Up @@ -1799,6 +1807,9 @@ HttpSM::state_http_server_open(int event, void *data)
HTTP_INCREMENT_DYN_STAT(http_origin_connections_throttled_stat);
send_origin_throttled_response();
} else {
// Go ahead and release the failed server session. Since it didn't receive a response, the release logic will
// see that it didn't get a valid response and it will close it rather than returning it to the server session pool
release_server_session();
call_transact_and_set_next_state(HttpTransact::HandleResponse);
}
return 0;
Expand Down Expand Up @@ -2285,6 +2296,7 @@ HttpSM::state_hostdb_reverse_lookup(int event, void *data)
int
HttpSM::state_mark_os_down(int event, void *data)
{
STATE_ENTER(&HttpSM::state_mark_os_down, event);
HostDBInfo *mark_down = nullptr;

if (event == EVENT_HOST_DB_LOOKUP && data) {
Expand Down Expand Up @@ -5401,7 +5413,7 @@ HttpSM::handle_http_server_open()
(t_state.hdr_info.request_content_length > 0 || t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING) &&
do_post_transform_open()) {
do_setup_post_tunnel(HTTP_TRANSFORM_VC);
} else {
} else if (server_session != nullptr) {
setup_server_send_request_api();
}
}
Expand Down Expand Up @@ -6863,7 +6875,10 @@ HttpSM::kill_this()
// then the value of kill_this_async_done has changed so
// we must check it again
if (kill_this_async_done == true) {
ink_assert(pending_action == nullptr);
if (pending_action) {
pending_action->cancel();
pending_action = nullptr;
}
if (t_state.http_config_param->enable_http_stats) {
update_stats();
}
Expand Down