Skip to content
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.

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

connection_pool: proper execution semantics for thread_safe=false #361

Open
anarthal opened this issue Oct 3, 2024 · 0 comments
Open

connection_pool: proper execution semantics for thread_safe=false #361

anarthal opened this issue Oct 3, 2024 · 0 comments
Labels
enhancement New feature or request

Comments

@anarthal
Copy link
Collaborator

anarthal commented Oct 3, 2024

While async_get_connection respects the usual Asio semantics for pools with thread_safe=false, connection tasks, which are technically children agents of async_run, do not. The proper semantics should be:

  • For thread_safe=false:
    • async_run intermediate handlers (e.g. notification timers): usual Asio semantics (as is today)
    • Connection task final handlers: since these affect pool state, usual Asio semantics (i.e. if async_run token has a bound executor, use this; otherwise, use the pool's executor). Currently, we always use the pool's executor. EDIT: these should always use the pool's executor, as it is today.
    • Connection task intermediate handlers (e.g. the ones run by connection::async_connect): if async_run token has a bound executor, use this. Otherwise, use the connection's executor. EDIT: these should always use the pool's executor, as it is today.
  • For thread_safe=true:
    • async_run intermediate handlers (e.g. notification timers): use the strand (as is today)
    • Connection task final handlers: use the strand (as is today)
    • Connection task intermediate handlers: there's no need to use the strand here, as these don't access pool state. EDIT: these can cause race conditions on the connections, too, so should use the pool's executor.

These should be documented.

EDIT: following the discussion in Slack, and given that using async_run token's for connection tasks is difficult, the current approach is good, but should be documented.

@anarthal anarthal added the enhancement New feature or request label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant