diff options
| -rw-r--r-- | cpp/CHANGES | 2 | ||||
| -rw-r--r-- | cpp/src/Ice/Connection.cpp | 60 | ||||
| -rw-r--r-- | cpp/src/Ice/Connection.h | 5 | ||||
| -rw-r--r-- | java/CHANGES | 2 | ||||
| -rw-r--r-- | java/src/IceInternal/Connection.java | 46 |
5 files changed, 57 insertions, 58 deletions
diff --git a/cpp/CHANGES b/cpp/CHANGES index 07d0162ed73..e8788ed673e 100644 --- a/cpp/CHANGES +++ b/cpp/CHANGES @@ -1,6 +1,8 @@ Changes since version 1.1.1 --------------------------- +- Fixed a crash that could happen during shutdown. + - Fixed a deadlock that could happen during connection establishment. - Fixed deadlock during shutdown that can happen if a thread pool with diff --git a/cpp/src/Ice/Connection.cpp b/cpp/src/Ice/Connection.cpp index 74554edddb8..cfb84752654 100644 --- a/cpp/src/Ice/Connection.cpp +++ b/cpp/src/Ice/Connection.cpp @@ -236,7 +236,6 @@ IceInternal::Connection::isValidated() const { // See _queryMutex comment in header file. IceUtil::Mutex::Lock sync(_queryMutex); - return _state > StateNotValidated; } @@ -245,7 +244,6 @@ IceInternal::Connection::isDestroyed() const { // See _queryMutex comment in header file. IceUtil::Mutex::Lock sync(_queryMutex); - return _state >= StateClosing; } @@ -254,13 +252,7 @@ IceInternal::Connection::isFinished() const { // See _queryMutex comment in header file. IceUtil::Mutex::Lock sync(_queryMutex); - - // - // If _transceiver is 0, then _dispatchCount must also be 0; - // - assert(!(_transceiver == 0 && _dispatchCount != 0)); - - return _transceiver == 0; + return _transceiver == 0 && _dispatchCount == 0; } void @@ -771,17 +763,22 @@ IceInternal::Connection::sendResponse(BasicStream* os, Byte compressFlag) try { - if(_state == StateClosed) { - assert(_dispatchCount == 0); - return; + // See _queryMutex comment in header file. + IceUtil::Mutex::Lock sync(_queryMutex); + --_dispatchCount; } - - if(--_dispatchCount == 0) + + if(_dispatchCount == 0) { notifyAll(); } + if(_state == StateClosed) + { + return; + } + bool compress; if(os->b.size() < 100) // Don't compress if message size is smaller than 100 bytes. { @@ -857,17 +854,22 @@ IceInternal::Connection::sendNoResponse() try { - if(_state == StateClosed) { - assert(_dispatchCount == 0); - return; + // See _queryMutex comment in header file. + IceUtil::Mutex::Lock sync(_queryMutex); + --_dispatchCount; } - - if(--_dispatchCount == 0) + + if(_dispatchCount == 0) { notifyAll(); } + if(_state == StateClosed) + { + return; + } + if(_state == StateClosing && _dispatchCount == 0) { initiateShutdown(); @@ -1059,7 +1061,11 @@ IceInternal::Connection::message(BasicStream& stream, const ThreadPoolPtr& threa traceRequest("received request", stream, _logger, _traceLevels); stream.read(requestId); invoke = 1; - ++_dispatchCount; + { + // See _queryMutex comment in header file. + IceUtil::Mutex::Lock sync(_queryMutex); + ++_dispatchCount; + } } break; } @@ -1080,7 +1086,11 @@ IceInternal::Connection::message(BasicStream& stream, const ThreadPoolPtr& threa { throw NegativeSizeException(__FILE__, __LINE__); } - _dispatchCount += invoke; + { + // See _queryMutex comment in header file. + IceUtil::Mutex::Lock sync(_queryMutex); + _dispatchCount += invoke; + } } break; } @@ -1406,6 +1416,8 @@ IceInternal::Connection::Connection(const InstancePtr& instance, IceInternal::Connection::~Connection() { + // See _queryMutex comment in header file. + IceUtil::Mutex::Lock sync(_queryMutex); assert(_state == StateClosed); assert(!_transceiver); assert(_dispatchCount == 0); @@ -1527,12 +1539,6 @@ IceInternal::Connection::setState(State state) case StateClosed: { // - // If we do a hard close, all outstanding requests are - // disregarded. - // - _dispatchCount = 0; - - // // If we change from not validated, we can close right // away. Otherwise we first must make sure that we are // registered, then we unregister, and let finished() do diff --git a/cpp/src/Ice/Connection.h b/cpp/src/Ice/Connection.h index 2198fafbb83..3868ad2d9ad 100644 --- a/cpp/src/Ice/Connection.h +++ b/cpp/src/Ice/Connection.h @@ -166,8 +166,9 @@ private: BasicStream _batchStream; int _batchRequestNum; - int _dispatchCount; // The number of requests currently being dispatched. - int _proxyCount; // The number of proxies using this connection. + int _dispatchCount; + + int _proxyCount; State _state; diff --git a/java/CHANGES b/java/CHANGES index 841d2ed4fe1..cbf7bcd4bdc 100644 --- a/java/CHANGES +++ b/java/CHANGES @@ -1,6 +1,8 @@ Changes since version 1.1.1 --------------------------- +- Fixed a crash that could happen during shutdown. + - Fixed a deadlock that could happen during connection establishment. - Fixed deadlock during shutdown that can happen if a thread pool with diff --git a/java/src/IceInternal/Connection.java b/java/src/IceInternal/Connection.java index 6a11df5e324..0f0ed06b546 100644 --- a/java/src/IceInternal/Connection.java +++ b/java/src/IceInternal/Connection.java @@ -237,17 +237,12 @@ public final class Connection extends EventHandler isFinished() { // - // No synchronization necessary, _transceiver is declared - // volatile. Synchronization is not possible here anyway, - // because this function must not block. - // - + // No synchronization necessary, _transceiver and + // _dispatchCount are declared volatile. Synchronization is + // not possible here anyway, because this function must not + // block. // - // If _transceiver is null, then _dispatchCount must also be 0; - // - assert(!(_transceiver == null && _dispatchCount != 0)); - - return _transceiver == null; + return _transceiver == null && _dispatchCount == 0; } public synchronized void @@ -679,15 +674,14 @@ public final class Connection extends EventHandler { try { - if(_state == StateClosed) + if(--_dispatchCount == 0) { - assert(_dispatchCount == 0); - return; + notifyAll(); } - if(--_dispatchCount == 0) + if(_state == StateClosed) { - notifyAll(); + return; } // @@ -724,17 +718,16 @@ public final class Connection extends EventHandler { try { - if(_state == StateClosed) - { - assert(_dispatchCount == 0); - return; - } - if(--_dispatchCount == 0) { notifyAll(); } + if(_state == StateClosed) + { + return; + } + if(_state == StateClosing && _dispatchCount == 0) { initiateShutdown(); @@ -1312,12 +1305,6 @@ public final class Connection extends EventHandler case StateClosed: { // - // If we do a hard close, all outstanding requests are - // disregarded. - // - _dispatchCount = 0; - - // // If we change from not validated, we can close right // away. Otherwise we first must make sure that we are // registered, then we unregister, and let finished() @@ -1530,8 +1517,9 @@ public final class Connection extends EventHandler private boolean _batchStreamInUse; private int _batchRequestNum; - private int _dispatchCount; // The number of requests currently being dispatched. - private int _proxyCount; // The number of proxies using this connection. + private volatile int _dispatchCount; // Must be volatile, see comment in isDestroyed(). + + private int _proxyCount; private volatile int _state; // Must be volatile, see comment in isDestroyed(). |