From 49c1c9a5f34f0060d1832b78a23e4732c6812e8b Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 12:11:38 -0500 Subject: [PATCH 1/6] Remove IceUtil::Mutex from Ruby --- ruby/src/IceRuby/ValueFactoryManager.cpp | 10 +++++----- ruby/src/IceRuby/ValueFactoryManager.h | 4 +++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ruby/src/IceRuby/ValueFactoryManager.cpp b/ruby/src/IceRuby/ValueFactoryManager.cpp index 0613bfd99e9..1f28638f239 100644 --- a/ruby/src/IceRuby/ValueFactoryManager.cpp +++ b/ruby/src/IceRuby/ValueFactoryManager.cpp @@ -88,7 +88,7 @@ IceRuby::ValueFactoryManager::add(Ice::ValueFactoryFunc, const string&) void IceRuby::ValueFactoryManager::add(const Ice::ValueFactoryPtr& f, const string& id) { - Lock lock(*this); + std::lock_guard lock(_mutex); if(id.empty()) { @@ -129,7 +129,7 @@ IceRuby::ValueFactoryManager::find(const string& id) const noexcept Ice::ValueFactoryPtr IceRuby::ValueFactoryManager::findCore(const string& id) const noexcept { - Lock lock(*this); + std::lock_guard lock(_mutex); if(id.empty()) { @@ -176,7 +176,7 @@ IceRuby::ValueFactoryManager::findValueFactory(const string& id) const void IceRuby::ValueFactoryManager::mark() { - Lock lock(*this); + std::lock_guard lock(_mutex); for(FactoryMap::iterator p = _factories.begin(); p != _factories.end(); ++p) { @@ -196,7 +196,7 @@ IceRuby::ValueFactoryManager::markSelf() volatile VALUE self; { - Lock lock(*this); + std::lock_guard lock(_mutex); self = _self; } @@ -219,7 +219,7 @@ IceRuby::ValueFactoryManager::destroy() FactoryMap factories; { - Lock lock(*this); + std::lock_guard lock(_mutex); if(_self == Qnil) { // diff --git a/ruby/src/IceRuby/ValueFactoryManager.h b/ruby/src/IceRuby/ValueFactoryManager.h index 5f31888755b..6fc5ba7d43d 100644 --- a/ruby/src/IceRuby/ValueFactoryManager.h +++ b/ruby/src/IceRuby/ValueFactoryManager.h @@ -55,7 +55,7 @@ class DefaultValueFactory : public Ice::ValueFactory }; using DefaultValueFactoryPtr = std::shared_ptr; -class ValueFactoryManager final : public Ice::ValueFactoryManager, public IceUtil::Mutex +class ValueFactoryManager final : public Ice::ValueFactoryManager { public: @@ -88,6 +88,8 @@ class ValueFactoryManager final : public Ice::ValueFactoryManager, public IceUti VALUE _self; FactoryMap _factories; DefaultValueFactoryPtr _defaultFactory; + + mutable std::mutex _mutex; }; using ValueFactoryManagerPtr = std::shared_ptr; From 3388b6f608a5e48f7383dc52ab151d0f1c6838a8 Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 12:16:05 -0500 Subject: [PATCH 2/6] Fix bogus assert --- python/modules/IcePy/ObjectAdapter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/modules/IcePy/ObjectAdapter.cpp b/python/modules/IcePy/ObjectAdapter.cpp index c6857b7ea91..623d5b9d8d5 100644 --- a/python/modules/IcePy/ObjectAdapter.cpp +++ b/python/modules/IcePy/ObjectAdapter.cpp @@ -588,7 +588,7 @@ adapterWaitForDeactivate(ObjectAdapterObject* self, PyObject* args) } } - assert(self->shutdown); + assert(self->deactivated); if(self->deactivateException) { setPythonException(*self->deactivateException); From bf12c1cd1cf78773d8e11e7ed8f9d917e4bee66d Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 13:19:19 -0500 Subject: [PATCH 3/6] windows ci: only build target language (#1682) --- .github/actions/build/action.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index c73af8ac651..9fb76b2e844 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -3,6 +3,7 @@ name: Build Ice runs: using: "composite" steps: + # Linux / macOS - name: Build C++ Dependencies run: make -j3 V=1 -C cpp srcs shell: bash @@ -16,12 +17,21 @@ runs: if: (runner.os == 'macOS' || runner.os == 'Linux') && (matrix.language == 'php' || matrix.language == 'js' || matrix.language == 'ruby') - name: Build ${{ matrix.language }} + working-directory: ./${{ matrix.language }} run: | - make -j3 V=1 LANGUAGES="${{ matrix.language }}" + make -j3 V=1 shell: bash if: runner.os == 'macOS' || runner.os == 'Linux' - - name: Build - run: msbuild /m /p:Platform=x64 ice.proj + # Windows + - name: Build C++ Dependencies + run: msbuild /m /p:Platform=x64 msbuild/ice.proj + working-directory: ./cpp + shell: powershell + if: runner.os == 'Windows' + + - name: Build ${{ matrix.language }} + run: msbuild /m /p:Platform=x64 msbuild/ice.proj + working-directory: ./${{ matrix.language }} shell: powershell if: runner.os == 'Windows' From 9e40ca31746ef04c618b8ed556019ffbcd594bb5 Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 13:48:02 -0500 Subject: [PATCH 4/6] Fix mutex include in python ValueFactoryManager.h --- python/modules/IcePy/ValueFactoryManager.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/modules/IcePy/ValueFactoryManager.h b/python/modules/IcePy/ValueFactoryManager.h index 168fab6e5c3..1c42c0640bb 100644 --- a/python/modules/IcePy/ValueFactoryManager.h +++ b/python/modules/IcePy/ValueFactoryManager.h @@ -7,7 +7,8 @@ #include #include -#include + +#include namespace IcePy { From d42a66985d3eb9f062e7335618996eaaa5fbb04f Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 13:49:08 -0500 Subject: [PATCH 5/6] Remove IceUtil::Mutex from objc (#1684) --- objective-c/src/Ice/Initialize.mm | 29 ++++++++++------------------- objective-c/src/Ice/PropertiesI.h | 2 +- objective-c/src/Ice/PropertiesI.mm | 4 ++-- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/objective-c/src/Ice/Initialize.mm b/objective-c/src/Ice/Initialize.mm index 4a753fc43f8..9e6735ab557 100644 --- a/objective-c/src/Ice/Initialize.mm +++ b/objective-c/src/Ice/Initialize.mm @@ -47,9 +47,10 @@ namespace { -typedef std::map CompactIdMap; +using CompactIdMap = std::map; -IceUtil::Mutex* _compactIdMapMutex = 0; +// The std::mutex consturctor is constexpr so this mutex is statically initialized +std::mutex _compactIdMapMutex; CompactIdMap* _compactIdMap = 0; class CompactIdResolverI : public Ice::CompactIdResolver @@ -59,10 +60,9 @@ virtual ::std::string resolve(::Ice::Int value) const { - assert(_compactIdMapMutex); assert(_compactIdMap); - IceUtilInternal::MutexPtrLock lock(_compactIdMapMutex); + std::lock_guard lock(_compactIdMapMutex); CompactIdMap::iterator p = _compactIdMap->find(value); if(p != _compactIdMap->end()) { @@ -73,8 +73,7 @@ }; // -// We don't use the constructor to initialize the compactIdMap as -// static initializtion takes place after +load is called. +// We don't use the constructor to initialize the compactIdMap as the initializtion takes place after +load is called. // class Init { @@ -87,12 +86,6 @@ delete _compactIdMap; _compactIdMap = 0; } - - if(_compactIdMapMutex) - { - delete _compactIdMapMutex; - _compactIdMapMutex = 0; - } } }; Init init; @@ -102,18 +95,15 @@ @implementation CompactIdMapHelper +(void) initialize { - assert(!_compactIdMapMutex); assert(!_compactIdMap); - _compactIdMapMutex = new ::IceUtil::Mutex; _compactIdMap = new CompactIdMap; } +(void) registerClass:(NSString*)type value:(ICEInt)value { - assert(_compactIdMapMutex); assert(_compactIdMap); - IceUtilInternal::MutexPtrLock lock(_compactIdMapMutex); + std::lock_guard lock(_compactIdMapMutex); CompactIdMap::iterator p = _compactIdMap->find(value); if(p != _compactIdMap->end()) { @@ -126,7 +116,7 @@ +(void) registerClass:(NSString*)type value:(ICEInt)value namespace IceObjC { -class ThreadNotification : public Ice::ThreadNotification, public IceUtil::Mutex +class ThreadNotification : public Ice::ThreadNotification { public: @@ -136,13 +126,13 @@ +(void) registerClass:(NSString*)type value:(ICEInt)value virtual void start() { - Lock sync(*this); + std::lock_guard lock(_mutex); _pools.insert(std::make_pair(IceUtil::ThreadControl().id(), [[NSAutoreleasePool alloc] init])); } virtual void stop() { - Lock sync(*this); + std::lock_guard lock(_mutex); std::map::iterator p = _pools.find(IceUtil::ThreadControl().id()); [p->second drain]; @@ -152,6 +142,7 @@ virtual void stop() private: std::map _pools; + std::mutex _mutex; }; }; diff --git a/objective-c/src/Ice/PropertiesI.h b/objective-c/src/Ice/PropertiesI.h index 0a5659ed20d..625b467e6c7 100644 --- a/objective-c/src/Ice/PropertiesI.h +++ b/objective-c/src/Ice/PropertiesI.h @@ -21,7 +21,7 @@ @interface ICENativePropertiesAdmin : ICEServantWrapper { - IceUtil::Mutex mutex_; + std::mutex mutex_; std::vector>> callbacks_; } @end diff --git a/objective-c/src/Ice/PropertiesI.mm b/objective-c/src/Ice/PropertiesI.mm index bb73c844c88..1a947ede858 100644 --- a/objective-c/src/Ice/PropertiesI.mm +++ b/objective-c/src/Ice/PropertiesI.mm @@ -225,7 +225,7 @@ -(void) load:(NSString*)file @implementation ICENativePropertiesAdmin -(void) addUpdateCallback:(id)cb { - IceUtil::Mutex::Lock sync(mutex_); + std::lock_guard lock(mutex_); std::function remover = std::dynamic_pointer_cast(object_)->addUpdateCallback( [cb](const Ice::PropertyDict& properties) @@ -254,7 +254,7 @@ -(void) addUpdateCallback:(id)cb -(void) removeUpdateCallback:(id)cb { - IceUtil::Mutex::Lock sync(mutex_); + std::lock_guard lock(mutex_); // Each removeUpdateCallback only removes the first occurrence auto p = std::find_if(callbacks_.begin(), callbacks_.end(), [cb](const auto& q) { return q.first == cb; }); From 3df69f43e4e825ebbe39b4dadbaa34b52ab095cd Mon Sep 17 00:00:00 2001 From: Joe George Date: Wed, 17 Jan 2024 13:49:21 -0500 Subject: [PATCH 6/6] Remove IceUtil::Mutex from php (#1685) --- php/src/Communicator.cpp | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/php/src/Communicator.cpp b/php/src/Communicator.cpp index 2c8bc722864..1b4eaf9e974 100644 --- a/php/src/Communicator.cpp +++ b/php/src/Communicator.cpp @@ -218,7 +218,9 @@ const string _defaultProfileName = ""; // typedef map RegisteredCommunicatorMap; RegisteredCommunicatorMap _registeredCommunicators; -IceUtil::Mutex* _registeredCommunicatorsMutex = 0; + +// std::mutex constructor is constexpr so it is statically initialized +std::mutex _registeredCommunicatorsMutex; IceUtil::TimerPtr _timer; @@ -228,24 +230,6 @@ IceUtil::TimerPtr _timer; // been used) by the request. // typedef map CommunicatorMap; - -class Init -{ -public: - - Init() - { - _registeredCommunicatorsMutex = new IceUtil::Mutex(); - } - - ~Init() - { - delete _registeredCommunicatorsMutex; - _registeredCommunicatorsMutex = 0; - } -}; - -Init init; } extern "C" @@ -335,7 +319,7 @@ ZEND_METHOD(Ice_Communicator, destroy) // Remove all registrations. // { - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); for(vector::iterator p = _this->ac->ids.begin(); p != _this->ac->ids.end(); ++p) { _registeredCommunicators.erase(*p); @@ -1260,7 +1244,7 @@ ZEND_FUNCTION(Ice_register) CommunicatorInfoIPtr info = Wrapper::value(comm); assert(info); - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); RegisteredCommunicatorMap::iterator p = _registeredCommunicators.find(id); if(p != _registeredCommunicators.end()) @@ -1312,7 +1296,7 @@ ZEND_FUNCTION(Ice_unregister) string id(s, sLen); - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); RegisteredCommunicatorMap::iterator p = _registeredCommunicators.find(id); if(p == _registeredCommunicators.end()) @@ -1347,7 +1331,7 @@ ZEND_FUNCTION(Ice_find) string id(s, sLen); - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); RegisteredCommunicatorMap::iterator p = _registeredCommunicators.find(id); if(p == _registeredCommunicators.end()) @@ -1802,7 +1786,7 @@ IcePHP::communicatorShutdown(void) { _profiles.clear(); - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); if(_timer) { @@ -2198,7 +2182,7 @@ IcePHP::ValueFactoryManager::destroy() void IcePHP::ReaperTask::runTimerTask() { - IceUtilInternal::MutexPtrLock lock(_registeredCommunicatorsMutex); + lock_guard lock(_registeredCommunicatorsMutex); IceUtil::Time now = IceUtil::Time::now(); RegisteredCommunicatorMap::iterator p = _registeredCommunicators.begin();