On 02/25/2015 11:52 AM, Rafael Schloming wrote:
On Wed, Feb 25, 2015 at 10:49 AM, Ted Ross <tr...@redhat.com> wrote:
Would it be safe to assume that any operations on driver->io are not
thread safe?
Dispatch is a multi-threaded application. It looks to me as though
io->error is a resource shared across the threads in an unsafe way.
Interesting... so this is what the docs say:
/**
* A ::pn_io_t manages IO for a group of pn_socket_t handles. A
* pn_io_t object may have zero or one pn_selector_t selectors
* associated with it (see ::pn_io_selector()). If one is associated,
* all the pn_socket_t handles managed by a pn_io_t must use that
* pn_selector_t instance.
*
* The pn_io_t interface is single-threaded. All methods are intended
* to be used by one thread at a time, except that multiple threads
* may use:
*
* ::pn_write()
* ::pn_send()
* ::pn_recv()
* ::pn_close()
* ::pn_selector_select()
*
* provided at most one thread is calling ::pn_selector_select() and
* the other threads are operating on separate pn_socket_t handles.
*/
I claim that the commit-in-question violates the text above. Calls to
pn_send() and pn_recv() are no longer thread-safe because they now use
the shared error record.
I think this has been somewhat modified by the constraints from the windows
implementation, and I'm not sure I understand completely what the
constraints are there, or entirely what is being described above, but on
the posix front, the pn_io_t is little more than just a holder for an error
slot, and you should have one of these per thread. It shouldn't be a
problem to use send/recv/etc from multiple threads though so long as you
pass in the pn_io_t from the current thread.
It's not desirable to allocate sockets to threads up front (i.e.
partition the set of sockets into per-thread slots). I know you didn't
say that was needed but it's what I infer from the docs for pn_io_t.
Assuming, as you suggest, that pn_io_t is nothing more than a
thread-specific error notepad seems like a recipe for future disaster
because pn_io_t is clearly intended to be more than that.
-Ted
