diff options
| -rw-r--r-- | swift/src/IceImpl/BlobjectFacade.mm | 3 | ||||
| -rw-r--r-- | swift/src/IceImpl/LocalObject.mm | 53 |
2 files changed, 42 insertions, 14 deletions
diff --git a/swift/src/IceImpl/BlobjectFacade.mm b/swift/src/IceImpl/BlobjectFacade.mm index 446535fdd30..f4f6189f9ce 100644 --- a/swift/src/IceImpl/BlobjectFacade.mm +++ b/swift/src/IceImpl/BlobjectFacade.mm @@ -26,6 +26,9 @@ BlobjectFacade::ice_invokeAsync(std::pair<const Byte*, const Byte*> inEncaps, ICEObjectAdapter* adapter = [ICEObjectAdapter getHandle:current.adapter]; ICEConnection* con = [ICEConnection getHandle:current.con]; + // Both are null (colloc) or both are non-null (non-colloc). + assert((current.con && con) || (!current.con && !con)); + @autoreleasepool { [_facade facadeInvoke:adapter diff --git a/swift/src/IceImpl/LocalObject.mm b/swift/src/IceImpl/LocalObject.mm index 092397a308a..7227ea71773 100644 --- a/swift/src/IceImpl/LocalObject.mm +++ b/swift/src/IceImpl/LocalObject.mm @@ -8,7 +8,8 @@ namespace { - std::unordered_map<void*, __weak ICELocalObject*> cachedObjects; + // We "leak" this map to avoid the destructor being called when the application is terminated. + auto* cachedObjects = new std::unordered_map<void*, __weak ICELocalObject*>(); } @implementation ICELocalObject @@ -26,8 +27,8 @@ namespace @synchronized([ICELocalObject class]) { - assert(cachedObjects.find(_cppObject.get()) == cachedObjects.end()); - cachedObjects.insert(std::make_pair(_cppObject.get(), self)); + assert(cachedObjects->find(_cppObject.get()) == cachedObjects->end()); + cachedObjects->insert(std::make_pair(_cppObject.get(), self)); } return self; } @@ -40,24 +41,48 @@ namespace } @synchronized([ICELocalObject class]) { - std::unordered_map<void*, __weak ICELocalObject*>::const_iterator p = cachedObjects.find(cppObject.get()); - if(p != cachedObjects.end()) + auto p = cachedObjects->find(cppObject.get()); + if (p != cachedObjects->end()) { - return p->second; - } - else - { - return [[[self class] alloc] initWithCppObject:std::move(cppObject)]; + // Get a strong reference to the object. If it's nil, preemptively remove it from the cache, + // otherwise we'll get an assert when we try to init a new one. + // This can happen if the object is being deallocated on another thread. + ICELocalObject* obj = p->second; + if (obj == nil) + { + cachedObjects->erase(p); + } + else + { + return obj; + } } + + return [[[self class] alloc] initWithCppObject:std::move(cppObject)]; } } --(void) dealloc { - assert(_cppObject != nullptr); +- (void)dealloc +{ @synchronized([ICELocalObject class]) { - cachedObjects.erase(_cppObject.get()); - _cppObject = nullptr; + assert(_cppObject != nullptr); + + // Find the object in the cache. If it's not there, it's likely that another thread already replaced + // the object with a new one and then that new one was quickly deallocated. + if (auto p = cachedObjects->find(_cppObject.get()); p != cachedObjects->end()) + { + // The object in the cache is either nil or NOT the current object. The later can happen if this thread was + // trying to deallocate the object while another thread was trying to create a new one. + assert(p->second == nil || p->second != self); + + // When the last reference on this object is released, p->second is nil and we remove the stale entry from + // the cache. + if (p->second == nil) + { + cachedObjects->erase(p); + } + } } } |