Skip to content

Conversation

locker
Copy link
Member

@locker locker commented Jun 22, 2022

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

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 tarantool#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 tarantool#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 tarantool#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.
Copy link
Collaborator

@Gerold103 Gerold103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

@locker locker assigned locker and unassigned Gerold103 Jun 24, 2022
@locker locker added the full-ci Enables all tests for a pull request label Jun 24, 2022
@locker locker merged commit 0d944f9 into tarantool:master Jun 24, 2022
@locker locker deleted the net-box-sync-request-in-trigger branch June 24, 2022 08:35
@locker
Copy link
Member Author

locker commented Jun 24, 2022

Cherry-picked to 2.10.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0005%) to 84.854% when pulling b92cea4 on locker:net-box-sync-request-in-trigger into 8d2bf38 on tarantool:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-ci Enables all tests for a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Net.box connection hangs on attempt to use call in on_connect trigger
3 participants