----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15677/#review29162 -----------------------------------------------------------
Ship it! The same fix is needed in SslTransport I believe. Though that may not be used on windows and may not actually show up he same crash, the code as is does have a race condition so even for Linux it is a bug in my view. - Gordon Sim On Nov. 19, 2013, 7:53 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15677/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2013, 7:53 p.m.) > > > Review request for qpid, Andrew Stitcher and Steve Huston. > > > Bugs: QPID-5363 > https://issues.apache.org/jira/browse/QPID-5363 > > > Repository: qpid > > > Description > ------- > > Windows fails with an access violation about 10% of the time while closing > AMQP 1.0 connections: > > STACK > ===== > > qpidmessagingd.dll!qpid::messaging::amqp::TcpTransport::close() > > Line 123 + 0x10 bytes C++ > qpidmessagingd.dll!qpid::messaging::amqp::ConnectionContext::close() > Line 134 + 0x29 bytes C++ > qpidmessagingd.dll!qpid::messaging::amqp::ConnectionHandle::close() > Line 62 C++ > qpidmessagingd.dll!qpid::messaging::Connection::close() Line 78 + > 0x24 bytes C++ > hello_world.exe!main(int argc=4, char * * argv=0x0053a688) Line 51 > + 0xe bytes C++ > hello_world.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C > hello_world.exe!mainCRTStartup() Line 403 C > > CODE > ==== > void TcpTransport::close() > { > QPID_LOG(debug, id << " TcpTransport closing..."); > if (aio) > aio->queueWriteClose(); <======= > } > > FAILURE > ======= > aio's vftable is all 0xdddddddd, indicating that it has been deleted. > > THE FIX > ======= > Use some locks to protect closing the aio object. > > DISCUSSION > ========== > The locks in the diff appear to work ok in that the code passes thousands of > runs. Are they OK theoretically? > > Include in 0.26 release > ----------------------- > Please indicate your approval or not in QPID-5363. > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/messaging/amqp/TcpTransport.h 1543532 > trunk/qpid/cpp/src/qpid/messaging/amqp/TcpTransport.cpp 1543532 > > Diff: https://reviews.apache.org/r/15677/diff/ > > > Testing > ------- > > Running HelloWorld in a loop in multiple windows works ok. > > Passes unit tests > > > Thanks, > > Chug Rolke > >
