net.box: explicitly forbid synchronous requests in triggers

Net.box triggers (on_connect, on_schema_reload) are executed
by the net.box connection worker fiber so a request issued by
a trigger callback can't be processed until the trigger returns
execution to the net.box fiber. Currently, an attempt to issue
a synchronous request from a net.box trigger leads to a silent
hang of the connection, which is confusing. Let's instead raise
an error until #7291 is implemented.

We need to add the check to three places in the code:
 1. luaT_netbox_wait_result for future:wait_result()
 2. luaT_netbox_iterator_next for future:pairs()
 3. conn._request for all synchronous requests.
    (We can't add the check to luaT_netbox_transport_perform_request,
    because conn._request may also call conn.wait_state, which would
    hang if called from on_connect or on_schema_reload trigger.)

We also add an assertion to netbox_request_wait to ensure that we
never wait for a request completion in the net.box worker fiber.

Closes #5358

@TarantoolBot document
Title: Synchronous requests are not allowed in net.box triggers

An attempt to issue a synchronous request (e.g. `call`) from
a net.box trigger (`on_connect`, `on_schema_reload`) now raises
an error: "Synchronous requests are not allowed in net.box trigger"
(Before #5358 was
fixed, it silently hung.)

Invoking an asynchronous request (see `is_async` option) is allowed,
but the request will not be processed until the trigger returns and
an attempt to wait for the request completion with `future:pairs()`
or `future:wait_result()` will raise the same error.

Author: locker

changelogs/unreleased/gh-5358-net-box-sync-request-in-trigger.md

@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed a hang when a synchronous request is issued from a net.box `on_connect`
+  or `on_schema_reload` trigger. Now, an error is raised instead (gh-5358).

src/box/lua/net_box.c

@@ -415,6 +415,12 @@ netbox_request_complete(struct netbox_request *request)
 static inline bool
 netbox_request_wait(struct netbox_request *request, double *timeout)
 {
+	/*
+	 * Waiting for a request completion in the net.box worker fiber
+	 * would result in a dead lock.
+	 */
+	assert(request->transport != NULL &&
+	       request->transport->worker != fiber());
 	if (*timeout == 0)
 		return false;
 	double ts = ev_monotonic_now(loop());
@@ -1877,6 +1883,11 @@ luaT_netbox_request_wait_result(struct lua_State *L)
 		    (timeout = lua_tonumber(L, 2)) < 0)
 			luaL_error(L, "Usage: future:wait_result(timeout)");
 	}
+	if (request->transport != NULL &&
+	    request->transport->worker == fiber()) {
+		luaL_error(L, "Synchronous requests are not allowed in "
+			   "net.box trigger");
+	}
 	while (!netbox_request_is_ready(request)) {
 		if (!netbox_request_wait(request, &timeout)) {
 			luaL_testcancel(L);
@@ -1927,6 +1938,11 @@ luaT_netbox_request_iterator_next(struct lua_State *L)
 	struct netbox_request *request = luaT_check_netbox_request(L, -1);
 	lua_rawgeti(L, 1, 2);
 	double timeout = lua_tonumber(L, -1);
+	if (request->transport != NULL &&
+	    request->transport->worker == fiber()) {
+		luaL_error(L, "Synchronous requests are not allowed in "
+			   "net.box trigger");
+	}
 	/* The second argument is the index of the last returned message. */
 	if (luaL_isnull(L, 2)) {
 		/* The previous call returned an error. */

src/box/lua/net_box.lua

@@ -274,6 +274,9 @@ local function new_sm(uri, opts)
     local last_reconnect_error
     local remote = {host = host, port = port, opts = opts, state = 'initial'}
     local function callback(what, ...)
+        if remote._fiber == nil then
+            remote._fiber = fiber.self()
+        end
         if what == 'state_changed' then
             local state, err = ...
             local was_connected = remote._is_connected
@@ -735,6 +738,9 @@ function remote_methods:_request(method, opts, format, stream_id, ...)
         on_push = on_push_sync_default
     end
     -- Execute synchronous request.
+    if self._fiber == fiber.self() then
+        error('Synchronous requests are not allowed in net.box trigger')
+    end
     local timeout = deadline and max(0, deadline - fiber_clock())
     if self.state ~= 'active' then
         self:wait_state('active', timeout)

test/box-luatest/net_box_test.lua

@@ -336,3 +336,35 @@ g.test_gc = function()
     t.assert_not(weak_refs.callback)
     t.assert_not(weak_refs.transport)
 end
+
+--
+-- Checks that synchronous requests fail with an error in on_connect and
+-- on_schema_reload triggers (gh-5358).
+--
+g.test_sync_request_in_trigger = function()
+    local c
+    local ch = fiber.channel(100)
+    local function trigger_cb()
+        local status, err
+        status, err = pcall(c.call, c, 'box.session.user', {timeout = 5})
+        ch:put(status or tostring(err))
+        local fut = c:call('box.session.user', {}, {is_async = true})
+        status, err = pcall(fut.wait_result, fut, 5)
+        ch:put(status or tostring(err))
+        status, err = pcall(function() for _, _ in fut:pairs(5) do end end)
+        ch:put(status or tostring(err))
+    end
+    local function check()
+        local msg = 'Synchronous requests are not allowed in net.box trigger'
+        t.assert_str_contains(ch:get(5), msg) -- conn:call
+        t.assert_str_contains(ch:get(5), msg) -- fut:wait_result
+        t.assert_str_contains(ch:get(5), msg) -- fut:pairs
+    end
+    c = net.connect(g.server.net_box_uri, {wait_connected = false})
+    t.assert_equals(c.state, 'initial')
+    c:on_connect(trigger_cb)
+    c:on_schema_reload(trigger_cb)
+    check() -- on_connect
+    check() -- on_schema_reload
+    c:close()
+end