Subject: [Boost-bugs] [Boost C++ Libraries] #10009: Boost Asio should provide also thread-safe sockets
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2014-05-06 14:51:27
#10009: Boost Asio should provide also thread-safe sockets
--------------------------------------------+----------------------------
Reporter: Vladislav Valtchev <vladi32@â¦> | Owner: chris_kohlhoff
Type: Feature Requests | Status: new
Milestone: To Be Determined | Component: asio
Version: Boost 1.55.0 | Severity: Problem
Keywords: |
--------------------------------------------+----------------------------
The feature I'm requesting is strictly related to the well-known problem
reported in ticket #8538, but I'm not reporting it as a bug since the
documentation explicitly states that the Asio's sockets are ''not''
thread-safe ''by design'': the user ''must'' use strands at top level to
ensure a correct behavior in a multi-threaded environment. If I've
correctly understood, the rationale for such design is that in a single
threaded environment, one should pay no cost for synchronization.
I disagree with this design decision for several reasons.
== The multi-threaded environments are more important ==
I believe that the environments where asio is seriously (heavily) used are
multi-threaded (there is an asio-based thread pool): this means that the
optimization is made for the simpler cases (where the cost for a not-
contended lock acquisition is negligible); IMHO this is wrong. This adds
an additional (significative) overhead to users which have necessarily to:
1. use strands (through strand::wrap() as suggested in the docs) ''or''
2. use mutexes (or other lock types) and carry always a reference to
them with the relative socket ''or''
3. wrap asio's socket with a custom class that internally uses locking.
== strand::wrap() is not always that useful ==
I believe that the solution officially recommended to "solve the problem"
(wrapping the callbacks with strand::wrap()) is theoretically a bad idea
since it causes the serialization (which is ''almost'' like what a mutex
does) of the callbacks invocation: this is ''deeply'' wrong because it is
an anti-pattern: the fundamental principle of synchronization is that ONLY
the data has to be "locked", NOT the code. Using strand::wrap() ( ~=
locking the CODE ) could lead to serious performance bottlenecks if the
callbacks' code is not ''carefully'' written (one could unconsciously do a
heavy computation (or sync I/O) through several levels of function calls).
So, within this model, writing an efficient code will mean to be careful
and to necessarily re-schedule all non-trivial tasks: that is not so easy
sometimes.[[br]]
What if, for example, one wants to close 100 sockets in a callback? Using
strand::wrap() would mean to use a common strand for all the 100 sockets
(= to serialize all the callbacks for these sockets): this is an
''unacceptable'' cost to pay. Instead, using a strand for each socket and
rescheduling a close() request using strand::post() is much better but
would mean always carrying back a reference of the relative strand for
each socket: this is same problem of the alternative 2: no one would be
happy to do this.[[br]]
It seems that the most attractive alternative is the third one.
== The non-thread-safe sockets are not something one would expect from
Asio ==
I'm pretty sure that I'm not the only one who, before reading
''carefully'' the documentation, was expecting that the sockets were ''of
course'' thread-safe: we are dealing with a high-level library for
asynchronous I/O. The situation is analogue to C++11's atomics: they, by
default, use FULL barriers because this way is simpler to use them;
advanced users (that really know what they're doing) instead, could always
specify the exact type of barrier they need. I think Asio is roughly at
the same level of abstraction of standard C++ libraries (if not at higher
one), so I think it should follow the same philosophy.
== Synchronization is already used in Asio's internals ==
Even if the official docs declare the asio's sockets as non-thread-safe,
actually ''there are'' synchronization mechanisms in asio's reactors
(epoll and kqueue) which allow the sockets to ''almost work'' in a multi-
threaded environment except when destructive operations like close() are
used. In these cases, the subtle race described in #8538 appears since the
mutex protecting the access to the descriptor_data node ''has to be''
released before "freeing it" because the mutex itself is a member of the
(same) node. That mutex makes me suspect that ''maybe'' an attempt has
been made to solve the synchronization problem at the reactor level but it
failed since the socket's lifetime is bigger than the descriptor_data node
which has to die when the descriptor is closed. Since the reactors has no
access to the upper level socket services (such thing will break the
isolation of the abstraction layers), the only solution seems to use a
top-level synchronization mechanism.
== What is my purpose to improve Asio ==
I purpose a simple patch (the attached file) that adds a lock contended
only among the async and the destructive operations at the top-level
classes (the *_socket_service classes). I think this solution is a good
compromise because all the code it adds is under an #ifdef: who wants to
work easily with asio's sockets has simply to specify it at compile-time;
the others could still pay the "zero" cost in single-threaded
applications. Even if I'd prefer the define to work in the opposite way,
I'm purposing this change because has a minor impact, conserves the old
behavior unless specified and so, I hope it has a greater chance to be
accepted.
== Just a final remark ==
I'd just like to say that, even if it probably doesn't emerge from my
comments, I'm an Asio's fan: overall, it is really a great boost-style
library and I use it intensively. That's the reason why I'm spending some
much energies trying to improve it.
-- Ticket URL: <https://svn.boost.org/trac/boost/ticket/10009> Boost C++ Libraries <http://www.boost.org/> Boost provides free peer-reviewed portable C++ source libraries.
This archive was generated by hypermail 2.1.7 : 2017-02-16 18:50:16 UTC