> On April 14, 2014, 7:57 p.m., Andrew Stitcher wrote: > > Can't see a problem with this change, but I'm wondering if you might need > > to check "state" before sending in the normal data path?
The current logic handles that case gracefully except "a queueWrite after a queueDelete" which doesn't happen from the transport layers. This eliminates that case (and only comes up on server side code in this one instance). The other cases based on (status == SEC_I_CONTINUE_NEEDED) won't occur in a SCHANNEL_SHUTDOWN context. As far as I can tell, the code up to and and including "state == Running" is pretty robust (aside from a TODO note about renegotiating). All the fireworks seem to happen on shutdown. This fix hopefully remains small and tied to the SSL logic, with more ambitious improvements listed in QPID-5668 deferred for a future release. - Cliff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20319/#review40305 ----------------------------------------------------------- On April 14, 2014, 3:44 p.m., Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20319/ > ----------------------------------------------------------- > > (Updated April 14, 2014, 3:44 p.m.) > > > Review request for qpid. > > > Bugs: QPID-5694 > https://issues.apache.org/jira/browse/QPID-5694 > > > Repository: qpid > > > Description > ------- > > This patch checks if the connection is closing before concluding that it has > a handshake packet to send. It appears this bug has existed a long time, but > the spurious packet was either dropped before sending or was ignored by the > client. The recent other changes for QPID-5669 has changed the timing of the > connection cleanup (sometimes sooner, sometimes later), allowing the problem > to now cause random broker errors reasonably frequently, whereas they were > infrequent or non-existent before. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/SslAsynchIO.cpp > 1587223 > > Diff: https://reviews.apache.org/r/20319/diff/ > > > Testing > ------- > > > Thanks, > > Cliff Jansen > >
