From 4d379127cb3356a843cc6a615c478644abf3204c Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@tarantool.org>
Date: Tue, 5 Dec 2023 13:31:28 +0300
Subject: [PATCH] iproto: don't account message twice in case of override
fallback
We need to call `tx_accept_msg` in `tx_process_override` before we pass
message to the override handler. Unfortunately if handler response with
IPROTO_HANDLER_FALLBACK we call the builtin handler for message that
calls `tx_accept_msg` again which is not expected. Some actions of
this function are idempotent and some are not.
Let's make the function NOP if it called once again.
Closes #9345
NO_DOC=bugfix
(cherry picked from commit 21112b066902338724f4f3671359f3e624b45938)
---
...-fix-iproto-override-double-request-account.md | 4 ++++
src/box/iproto.cc | 10 ++++++++++
.../iproto_request_handlers_overriding_test.lua | 15 +++++++++++++++
3 files changed, 29 insertions(+)
create mode 100644 changelogs/unreleased/gh-9345-fix-iproto-override-double-request-account.md
diff --git a/changelogs/unreleased/gh-9345-fix-iproto-override-double-request-account.md b/changelogs/unreleased/gh-9345-fix-iproto-override-double-request-account.md
new file mode 100644
index 0000000000..5198b47b5b
--- /dev/null
+++ b/changelogs/unreleased/gh-9345-fix-iproto-override-double-request-account.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed incorrect calculation of requests in progress in case of iproto
+ override fallback (gh-9345).
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index d963dfbbc1..4c37a869da 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -420,6 +420,12 @@ struct iproto_msg
struct stailq_entry in_stream;
/** Stream that owns this message, or NULL. */
struct iproto_stream *stream;
+ /**
+ * True if message processing in tx thread is started. The flag is
+ * used to prevent double accounting of message in case of iproto
+ * override fallback to original handler.
+ */
+ bool accepted;
};
static struct iproto_msg *
@@ -812,6 +818,7 @@ iproto_msg_new(struct iproto_connection *con)
msg->close_connection = false;
msg->connection = con;
msg->stream = NULL;
+ msg->accepted = false;
rmean_collect(con->iproto_thread->rmean, IPROTO_REQUESTS, 1);
return msg;
}
@@ -1978,6 +1985,9 @@ static inline struct iproto_msg *
tx_accept_msg(struct cmsg *m)
{
struct iproto_msg *msg = (struct iproto_msg *) m;
+ if (msg->accepted)
+ return msg;
+ msg->accepted = true;
tx_accept_wpos(msg->connection, &msg->wpos);
tx_fiber_init(msg->connection->session, msg->header.sync);
tx_prepare_transaction_for_request(msg);
diff --git a/test/box-luatest/iproto_request_handlers_overriding_test.lua b/test/box-luatest/iproto_request_handlers_overriding_test.lua
index 18731e06aa..4c7fdd6783 100644
--- a/test/box-luatest/iproto_request_handlers_overriding_test.lua
+++ b/test/box-luatest/iproto_request_handlers_overriding_test.lua
@@ -417,3 +417,18 @@ g.test_box_iproto_override_cb_lua_invalid_return_type = function(cg)
box.iproto.override(box.iproto.type.PING, nil)
end)
end
+
+-- gh-9345: Checks that we don't account message twice in case of fallback.
+g.test_box_iproto_override_fallback_double_accounting = function(cg)
+ cg.server:exec(function()
+ local cb_fallback = function()
+ return false
+ end
+ local before = box.stat.net().REQUESTS_IN_PROGRESS.current
+ box.iproto.override(box.iproto.type.PING, cb_fallback)
+ _G.test_ping()
+ box.iproto.override(box.iproto.type.PING, nil)
+ local after = box.stat.net().REQUESTS_IN_PROGRESS.current
+ t.assert_equals(after - before, 0)
+ end)
+end
--
GitLab