Discussion on thread-safety

doc/qbk/20_b_connection_pools.qbk

@@ -156,6 +156,35 @@ In short:
   Otherwise, it becomes `pending_connect` to be reconnected. Pings can be disabled by
   setting [refmem pool_params ping_interval] to zero.
 
+[endsect]
+
+[section Thread-safety and executors]
+
+By default, [reflink connection_pool] is [*NOT thread-safe], but it can
+be easily made thread-safe by using:
+
+[connection_pool_thread_safe]
+
+This works by using strands. Recall that a [asioreflink strand] is Asio's method to enable concurrency
+without explicit locking. A strand is an executor that wraps another executor.
+All handlers dispatched through a strand will be serialized: no two handlers
+will be run in parallel, which avoids data races.
+
+We're passing a [reflink pool_executor_params] instance to the pool's
+constructor, which contains two executors:
+
+* [refmem pool_executor_params pool_executor] is used to run [refmem connection_pool async_run]
+  and [refmem connection_pool async_get_connection] intermediate handlers. By using
+  [refmem pool_executor_params thread_safe], a strand is created, and all these handlers
+  will be serialized.
+* [refmem pool_executor_params connection_executor] is used to construct connections.
+  By default, this won't be wrapped in any strand, and inividual connections will not be thread-safe.
+
+
+
+
+
+
 [endsect]
 
 
@@ -169,16 +198,6 @@ In short:
 
 
 
-* Thread-safety. By default, `connection_pool` is thread-safe. You can use it in a MT context,
-  share a single instance between threads, without race conditions. How this works:
-  * The pool will internally create an `asio::strand` around the executor you pass to the pool's ctor.
-  * All functions are either inherently thread-safe (e.g. use `std::amotic<T>`), or will use
-    `asio::post`/`asio::dispatch` to ensure that all the intermediate handlers are run in a thread-safe manner.
-  * Individual connections returned by the pool are *NOT* thread-safe. You're responsible for
-    not incurring in race-conditions with them. They are constructed using the executor you pass.
-  * This thread-safety mechanism can be disabled using `enable_thread_safety=false`. It's enabled by
-    default. By disabling it, you're responsible for keeping thread-safety. Can be used to increase
-    performance or to build custom thread-safety mechanisms on top.
 
 * Transport types and SSL.
   * The connection pool uses `any_connection`, so it can be used with TCP, TCP with SSL and UNIX sockets.

example/snippets.cpp

@@ -1414,6 +1414,31 @@ void section_connection_pool(string_view server_hostname, string_view username,
         run_coro(ctx.get_executor(), [&pool] { return return_without_reset(pool); });
 #endif
     }
+    {
+        //[connection_pool_thread_safe
+        // The I/O context, required by all I/O operations
+        boost::asio::io_context ctx;
+
+        // The usual pool configuration params
+        boost::mysql::pool_params params;
+        params.server_address.emplace_host_and_port(server_hostname);
+        params.username = username;
+        params.password = password;
+        params.database = "boost_mysql_examples";
+
+        // By passing pool_executor_params::thread_safe to connection_pool,
+        // we make all its member functions thread-safe.
+        // This works by creating a strand
+        boost::mysql::connection_pool pool(
+            boost::mysql::pool_executor_params::thread_safe(ctx.get_executor()),
+            std::move(params)
+        );
+
+        // We can now pass a reference to pool to other threads,
+        // and call async_get_connection concurrently without problem.
+        // Inidivudal connections are still not thread-safe.
+        //]
+    }
 }
 
 void main_impl(int argc, char** argv)

include/boost/mysql/connection_pool.hpp

@@ -51,10 +51,10 @@ namespace mysql {
  * This is a move-only type.
  *
  * \par Thread-safety
- * By default, connection pools are *not* thread-safe, but they can
+ * By default, connection pools are *not* thread-safe, but most functions can
  * be made thread-safe by passing an adequate \ref pool_executor_params objects
  * to the constructor. See \ref pool_executor_params::thread_safe and the discussion
- * and examples for details.
+ * for details.
  * \n
  * Distinct objects: safe. \n
  * Shared objects: unsafe, unless passing adequate values to the constructor.
@@ -117,6 +117,11 @@ class connection_pool
      *
      * \par Exception safety
      * No-throw guarantee.
+     *
+     * \par Thead-safety
+     * This function is never thread-safe, regardless of the executor
+     * configuration passed to the constructor. Calling this function
+     * concurrently with any other function introduces data races.
      */
     connection_pool(connection_pool&& other) = default;
 
@@ -133,6 +138,11 @@ class connection_pool
      *
      * \par Exception safety
      * No-throw guarantee.
+     *
+     * \par Thead-safety
+     * This function is never thread-safe, regardless of the executor
+     * configuration passed to the constructor. Calling this function
+     * concurrently with any other function introduces data races.
      */
     connection_pool& operator=(connection_pool&& other) = default;
 
@@ -156,6 +166,11 @@ class connection_pool
      *
      * \par Exception safety
      * No-throw guarantee.
+     *
+     * \par Thead-safety
+     * This function is never thread-safe, regardless of the executor
+     * configuration passed to the constructor. Calling this function
+     * concurrently with any other function introduces data races.
      */
     bool valid() const noexcept { return impl_.get() != nullptr; }
 
@@ -170,6 +185,11 @@ class connection_pool
      *
      * \par Exception safety
      * No-throw guarantee.
+     *
+     * \par Thead-safety
+     * This function is never thread-safe, regardless of the executor
+     * configuration passed to the constructor. Calling this function
+     * concurrently with any other function introduces data races.
      */
     executor_type get_executor() noexcept { return impl_->get_executor(); }
 
@@ -204,6 +224,15 @@ class connection_pool
      * \par Errors
      * This function always complete successfully. The handler signature ensures
      * maximum compatibility with Boost.Asio infrastructure.
+     *
+     * \par Executor
+     * All intermediate completion handlers are dispatched through the pool's
+     * executor (as given by `this->get_executor()`).
+     *
+     * \par Thead-safety
+     * When the pool is constructed with adequate executor configuration, this function
+     * is safe to be called concurrently with \ref async_get_connection, \ref cancel,
+     * \ref pooled_connection::~pooled_connection and \ref pooled_connection::return_without_reset.
      */
     template <BOOST_ASIO_COMPLETION_TOKEN_FOR(void(::boost::mysql::error_code)) CompletionToken>
     BOOST_ASIO_INITFN_AUTO_RESULT_TYPE(CompletionToken, void(error_code))
@@ -295,6 +324,15 @@ class connection_pool
      *     (e.g. all connections are in use and limits forbid creating more).
      * \li \ref client_errc::cancelled if \ref cancel was called before or while
      *     the operation is outstanding, or if the pool is not running.
+     *
+     * \par Executor
+     * All intermediate completion handlers are dispatched through the pool's
+     * executor (as given by `this->get_executor()`).
+     *
+     * \par Thead-safety
+     * When the pool is constructed with adequate executor configuration, this function
+     * is safe to be called concurrently with \ref async_run, \ref cancel,
+     * \ref pooled_connection::~pooled_connection and \ref pooled_connection::return_without_reset.
      */
     template <
         BOOST_ASIO_COMPLETION_TOKEN_FOR(void(::boost::mysql::error_code, ::boost::mysql::pooled_connection))
@@ -331,6 +369,11 @@ class connection_pool
      *
      * \par Exception safety
      * Basic guarantee. Memory allocations and acquiring mutexes may throw.
+     *
+     * \par Thead-safety
+     * When the pool is constructed with adequate executor configuration, this function
+     * is safe to be called concurrently with \ref async_run, \ref async_get_connection,
+     * \ref pooled_connection::~pooled_connection and \ref pooled_connection::return_without_reset.
      */
     void cancel()
     {

include/boost/mysql/pooled_connection.hpp

@@ -111,6 +111,12 @@ class pooled_connection
      * If `this->valid() == true`, returns the owned connection to the pool
      * and marks it as pending reset. If your connection doesn't need to be reset
      * (e.g. because you didn't mutate session state), use \ref return_without_reset.
+     *
+     * \par Thead-safety
+     * If the \ref connection_pool object that `*this` references has been constructed
+     * with adequate executor configuration, this function is safe to be called concurrently
+     * with \ref connection_pool::async_run, \ref connection_pool::async_get_connection,
+     * \ref connection_pool::cancel and \ref return_without_reset on other `pooled_connection` objects.
      */
     ~pooled_connection() = default;
 
@@ -132,11 +138,6 @@ class pooled_connection
      *
      * \par Exception safety
      * No-throw guarantee.
-     *
-     * \par Thread-safety
-     * The returned connection doesn't have any synchronization mechanisms built-in.
-     * It is unsafe to call functions concurrently on the same object from different
-     * threads. Treat it as if it was a \ref any_connection you created manually.
      */
     any_connection& get() noexcept { return impl_->connection(); }
 
@@ -171,6 +172,12 @@ class pooled_connection
      *
      * \par Exception safety
      * No-throw guarantee.
+     *
+     * \par Thead-safety
+     * If the \ref connection_pool object that `*this` references has been constructed
+     * with adequate executor configuration, this function is safe to be called concurrently
+     * with \ref connection_pool::async_run, \ref connection_pool::async_get_connection,
+     * \ref connection_pool::cancel and \ref ~pooled_connection (on other objects).
      */
     void return_without_reset() noexcept
     {