Skip to content

Commit

Permalink
iproto: fix assertion on dropping of a new connection
Browse files Browse the repository at this point in the history
We need to handle case of dropping new connection. When
net_send_greeting() is executed the connection can be closed due to
iproto_drop_connections() call.

Note that in the test the Tarantool crashes for another reason. Due to
access after sleep to the connection that is destroyed so its memory is
poisoned. Yet we visit net_send_greeting() too in the test with patch so
original issue is verified too. We also need to test that such
a connection is closed. This will be done in EE version.

Closes tarantool#9717

NO_DOC=bugfix
  • Loading branch information
nshy authored and locker committed Feb 22, 2024
1 parent d592f26 commit a12998a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## bugfix/core

* Fixed a crash on dropping a just accepted connection (gh-9717).
18 changes: 17 additions & 1 deletion src/box/iproto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,8 @@ struct iproto_connection
* connection.
*/
struct cmsg cancel_msg;
/** Set if connection is accepted in TX. */
bool is_established;
};

/** Returns a string suitable for logging. */
Expand Down Expand Up @@ -1663,6 +1665,7 @@ iproto_connection_new(struct iproto_thread *iproto_thread)
con->session = NULL;
con->is_in_replication = false;
con->is_drop_pending = false;
con->is_established = false;
rlist_create(&con->in_stop_list);
rlist_create(&con->tx.inprogress);
rlist_add_entry(&iproto_thread->connections, con, in_connections);
Expand Down Expand Up @@ -3329,6 +3332,11 @@ net_send_greeting(struct cmsg *m)
{
struct iproto_msg *msg = (struct iproto_msg *) m;
struct iproto_connection *con = msg->connection;
if (con->is_drop_pending) {
iproto_connection_close(con);
iproto_msg_delete(msg);
return;
}
if (msg->close_connection) {
struct obuf *out = msg->wpos.obuf;
int64_t nwr = iostream_writev(&con->io, out->iov,
Expand All @@ -3345,6 +3353,7 @@ net_send_greeting(struct cmsg *m)
iproto_msg_delete(msg);
return;
}
con->is_established = true;
con->wend = msg->wpos;
/*
* Connect is synchronous, so no one could have been
Expand Down Expand Up @@ -3988,9 +3997,16 @@ iproto_do_cfg_f(struct cbus_call_msg *m)
* cannot close them as usual. Anyway we cancel
* replication fibers as well and close connection
* after replication is breaked.
*
* Do not close connection that is not yet
* established. Otherwise session
* on_connect/on_disconnect callbacks may be
* executed in reverse order in case of yields
* in on_connect callbacks.
*/
if (!con->is_in_replication &&
con->state == IPROTO_CONNECTION_ALIVE)
con->state == IPROTO_CONNECTION_ALIVE &&
con->is_established)
iproto_connection_close(con);
/*
* Do not wait deletion of connection that called
Expand Down
28 changes: 28 additions & 0 deletions test/box-luatest/shutdown_test.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
local server = require('luatest.server')
local utils = require('luatest.utils')
local justrun = require('test.justrun')
local fio = require('fio')
local popen = require('popen')
local fiber = require('fiber')
Expand Down Expand Up @@ -74,6 +75,33 @@ g_crash.test_shutdown_during_snapshot_on_signal = function(cg)
t.assert_equals(status.exit_code, 0)
end

-- Test shutdown when new iproto connection is accepted but
-- not yet fully established.
g_crash.test_shutdown_during_new_connection = function(cg)
local script = [[
local net = require('net.box')
local fiber = require('fiber')
local sock = 'unix/:./iproto.sock'
box.cfg{listen = sock}
local cond = fiber.cond()
local in_trigger = false
box.session.on_connect(function()
in_trigger = true
cond:signal()
fiber.sleep(1)
end)
net.connect(sock, {wait_connected = false})
while not in_trigger do
cond:wait()
end
os.exit()
]]
local result = justrun.tarantool(cg.workdir, {}, {'-e', script},
{nojson = true, quote_args = true})
t.assert_equals(result.exit_code, 0)
end

local g = t.group()

g.before_each(function(cg)
Expand Down

0 comments on commit a12998a

Please sign in to comment.