Hi Cliff, I hope you don't mind if I reply on list, and not in Jira...
On 12. 09. 14 09:18, Cliff Jansen (JIRA) wrote: > [ > https://issues.apache.org/jira/browse/PROTON-668?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14131213#comment-14131213 > ] > > Cliff Jansen commented on PROTON-668: > ------------------------------------- > > Additional notes based on comments to the Proton mailing list: > >>> a socket may only ever be used with a single pn_io_t in its lifetime >>> exception: a socket from pn_accept() is not yet associated with any >>> pn_io_t >>> and its first use can be with any pn_io_t (or never with a pn_io_t at >>> all) >> I'm not sure if it's implied, but pn_connect() and pn_listen() also need >> to support 'third party event loop'. >> Specifically, pn_connect() has to remain non-blocking (we get to know >> about the connect error later in the external event loop) > Yes third party loop support is implied for pn_listen and pn_connect. > The exception comment is about "moving" a socket between two pn_io_t > instances (or determining "first use"). The third party loop is merely > not allowed to pass any given socket to more that one pn_io_t. > OK. >>> threads may do concurrent pn_io_xx) calls as long as no two are >>> simultaneous >>> on the same socket (where xxx is send/recv/read/write) >> This will break pn_io_error() and pn_io_wouldblock() as they are defined now. > There is certainly no error or blocking state kept per socket, or per > thread. I am describing the existing multi-threaded use in driver.c > (heavily used by Dispatch) and assuming others may wish to do the same > on Windows. pn_io_error and pn_wouldblock are conspicuously absent in > driver.c (old and new). > How the new driver propagates errors? I saw it's looking at errno, but, does errno interact with iocp? (didn't look at that, sorry) Here is one example why sharing an pn_io_t across threads seems unsafe, even if those threads do not use pn_io_error() call: ssize_t pn_send(pn_io_t *io, pn_socket_t sockfd, const void *buf, size_t len) { ssize_t count; iocpdesc_t *iocpd = pni_iocpdesc_map_get(io->iocp, sockfd); if (iocpd) { count = pni_iocp_begin_write(iocpd, buf, len, &io->wouldblock, io->error); So if two threads do pn_send() on same pn_io_t but on two different pn_socket_t that are both in some error state (that is captured in their respective iocpdesc_t->error), both threads will access the same io->error, and that cannot end well. >> I have a feeling you don't really want/need to expose the pn_pipe(), >> but add a pn_selector_interrupt() and a mechanism of querying that for >> the caller of pn_selector_select() especially as you want to implement >> it completely differently on windows. > Yes, the pipe is an awkward difference between Windows and Posix > thinking, where the latter sees pipes and sockets as close kin but > Windows sees them as very distant relatives. > > Ditching the pn_pipe() call altogether in favour of a > pn_selector_interrupt() is cleanest and most performant from the > Windows IOCP perspective. However, as long as a pn_pipe is only > accessed via pn_read and pn_write the scope for optimization reduces > any performance penalty to a minimum. So I am OK keeping things as > they are but would not resist change. > > agree that starting with the smallest API change is better. Bozzo
