diff --git a/headers/os/app/AppDefs.h b/headers/os/app/AppDefs.h index 77383ef39a..0a70d8b432 100644 --- a/headers/os/app/AppDefs.h +++ b/headers/os/app/AppDefs.h @@ -59,6 +59,7 @@ enum { B_VALUE_CHANGED = '_VCH', B_TRANSLATOR_ADDED = '_ART', B_TRANSLATOR_REMOVED = '_RRT', + B_DELETE_TRANSLATOR = '_DRT', B_VIEW_MOVED = '_VMV', B_VIEW_RESIZED = '_VRS', B_WINDOW_MOVED = '_WMV', diff --git a/headers/os/translation/TranslatorRoster.h b/headers/os/translation/TranslatorRoster.h index 4d95853ed3..b01d9c2701 100644 --- a/headers/os/translation/TranslatorRoster.h +++ b/headers/os/translation/TranslatorRoster.h @@ -23,6 +23,17 @@ class BView; struct entry_ref; +class BTranslatorReleaseDelegate { +public: + BTranslatorReleaseDelegate(BTranslator* translator); + + void Release(); + +private: + BTranslator* fUnderlying; +}; + + class BTranslatorRoster : public BArchivable { public: BTranslatorRoster(); @@ -86,6 +97,8 @@ public: translator_id translatorID, BMessage* ioExtension); + BTranslatorReleaseDelegate* AcquireTranslator(int32 translatorID); + status_t GetRefFor(translator_id translatorID, entry_ref* ref); bool IsTranslator(entry_ref* ref); diff --git a/src/kits/translation/Translator.cpp b/src/kits/translation/Translator.cpp index 5699901c42..6afb80cfe0 100644 --- a/src/kits/translation/Translator.cpp +++ b/src/kits/translation/Translator.cpp @@ -24,8 +24,6 @@ BTranslator::BTranslator() BTranslator::~BTranslator() { - if (fOwningRoster != NULL) - fOwningRoster->TranslatorDeleted(fID); } @@ -52,10 +50,24 @@ BTranslator *BTranslator::Acquire() BTranslator *BTranslator::Release() { int32 oldValue = atomic_add(&fRefCount, -1); - if (oldValue > 0) + if (oldValue > 1) return this; - delete this; + if (fOwningRoster == NULL) { + delete this; + return NULL; + } + + // If we have ever been part of a roster, notify the roster to delete us + // and unload our image in a thread-safe way + BMessage deleteRequest(B_DELETE_TRANSLATOR); + + deleteRequest.AddPointer("ptr", this); + deleteRequest.AddInt32("id", fID); + + BMessenger sender(fOwningRoster); + sender.SendMessage(&deleteRequest); + return NULL; } diff --git a/src/kits/translation/TranslatorRoster.cpp b/src/kits/translation/TranslatorRoster.cpp index a0b54af4af..4e3f7c2c6e 100644 --- a/src/kits/translation/TranslatorRoster.cpp +++ b/src/kits/translation/TranslatorRoster.cpp @@ -193,9 +193,6 @@ BTranslatorRoster::Private::~Private() while (iterator != fTranslators.end()) { BTranslator* translator = iterator->second.translator; - translator->fOwningRoster = NULL; - // we don't want to be notified about this anymore - images.insert(iterator->second.image); translator->Release(); @@ -318,6 +315,19 @@ BTranslatorRoster::Private::MessageReceived(BMessage* message) break; } + case B_DELETE_TRANSLATOR: + { + // A translator's refcount has been reduced to zero and it wants + // us to delete it. + int32 id; + void* self; + if (message->FindInt32("id", &id) == B_OK + && message->FindPointer("ptr", &self) == B_OK) { + _TranslatorDeleted(id, (BTranslator*)self); + } + break; + } + default: BHandler::MessageReceived(message); break; @@ -592,6 +602,7 @@ BTranslatorRoster::Private::CreateTranslators(const entry_ref& ref, if (AddTranslator(translator, image, &ref, nodeRef.node) == B_OK) { if (update) update->AddInt32("translator_id", translator->fID); + fImageOrigins.insert(std::make_pair(translator, image)); count++; created++; } else { @@ -600,8 +611,12 @@ BTranslatorRoster::Private::CreateTranslators(const entry_ref& ref, } } - if (created == 0) + if (created == 0) { unload_add_on(image); + } else { + // Initial refcount for the image that was just loaded + fKnownImages.insert(std::make_pair(image, created)); + } quarantine.Remove(); return B_OK; @@ -850,18 +865,27 @@ BTranslatorRoster::Private::GetRefFor(translator_id id, entry_ref& ref) void -BTranslatorRoster::Private::TranslatorDeleted(translator_id id) +BTranslatorRoster::Private::_TranslatorDeleted(translator_id id, BTranslator* self) { BAutolock locker(this); TranslatorMap::iterator iterator = fTranslators.find(id); - if (iterator == fTranslators.end()) - return; + if (iterator != fTranslators.end()) + fTranslators.erase(iterator); - fTranslators.erase(iterator); + image_id image = fImageOrigins[self]; + + delete self; + + int32 former = atomic_add(&fKnownImages[image], -1); + if (former == 1) + { + unload_add_on(image); + fImageOrigins.erase(self); + fKnownImages.erase(image); + } } - /*static*/ int BTranslatorRoster::Private::_CompareSupport(const void* _a, const void* _b) { @@ -1073,9 +1097,6 @@ BTranslatorRoster::Private::_RemoveTranslators(const node_ref* nodeRef, if ((ref != NULL && item.ref == *ref) || (nodeRef != NULL && item.ref.device == nodeRef->device && item.node == nodeRef->node)) { - item.translator->fOwningRoster = NULL; - // if the translator is busy, we don't want to be notified - // about the removal later on item.translator->Release(); image = item.image; update.AddInt32("translator_id", iterator->first); @@ -1086,11 +1107,6 @@ BTranslatorRoster::Private::_RemoveTranslators(const node_ref* nodeRef, iterator = next; } - // Unload image from the removed translator - - if (image >= B_OK) - unload_add_on(image); - _NotifyListeners(update); } @@ -1150,6 +1166,21 @@ BTranslatorRoster::Private::_NotifyListeners(BMessage& update) const // #pragma mark - +BTranslatorReleaseDelegate::BTranslatorReleaseDelegate(BTranslator* translator) + : + fUnderlying(translator) +{ +} + + +void +BTranslatorReleaseDelegate::Release() +{ + fUnderlying->Release(); + // ReleaseDelegate is only allowed to release a translator once. + delete this; +} + BTranslatorRoster::BTranslatorRoster() { @@ -1665,6 +1696,20 @@ BTranslatorRoster::MakeConfigurationView(translator_id id, } +BTranslatorReleaseDelegate* +BTranslatorRoster::AcquireTranslator(int32 id) +{ + BAutolock locker(fPrivate); + + BTranslator* translator = fPrivate->FindTranslator(id); + if (translator == NULL) + return NULL; + + translator->Acquire(); + return new BTranslatorReleaseDelegate(translator); +} + + /*! Gets the configuration setttings for the translator specified by \a id and puts them into \a ioExtension. diff --git a/src/kits/translation/TranslatorRosterPrivate.h b/src/kits/translation/TranslatorRosterPrivate.h index dac2c01ef1..ceb9d3d94c 100644 --- a/src/kits/translation/TranslatorRosterPrivate.h +++ b/src/kits/translation/TranslatorRosterPrivate.h @@ -33,6 +33,8 @@ typedef std::map TranslatorMap; typedef std::vector MessengerList; typedef std::vector NodeRefList; typedef std::set EntryRefSet; +typedef std::map ImageMap; +typedef std::map TranslatorImageMap; class BTranslatorRoster::Private : public BHandler, public BLocker { @@ -78,8 +80,6 @@ public: status_t StartWatching(BMessenger target); status_t StopWatching(BMessenger target); - void TranslatorDeleted(translator_id id); - private: static int _CompareSupport(const void* _a, const void* _b); @@ -107,11 +107,15 @@ private: const char* name); void _EntryAdded(const entry_ref& ref); void _NotifyListeners(BMessage& update) const; + void _TranslatorDeleted(translator_id id, + BTranslator *self); NodeRefList fDirectories; TranslatorMap fTranslators; MessengerList fMessengers; EntryRefSet fRescanEntries; + ImageMap fKnownImages; + TranslatorImageMap fImageOrigins; const char* fABISubDirectory; int32 fNextID; bool fLazyScanning; diff --git a/src/preferences/datatranslations/DataTranslationsWindow.cpp b/src/preferences/datatranslations/DataTranslationsWindow.cpp index 24b83e58c3..c1ce98299c 100644 --- a/src/preferences/datatranslations/DataTranslationsWindow.cpp +++ b/src/preferences/datatranslations/DataTranslationsWindow.cpp @@ -53,7 +53,8 @@ DataTranslationsWindow::DataTranslationsWindow() : BWindow(BRect(0, 0, 550, 350), B_TRANSLATE_SYSTEM_NAME("DataTranslations"), B_TITLED_WINDOW, B_ASYNCHRONOUS_CONTROLS | B_NOT_ZOOMABLE - | B_NOT_RESIZABLE | B_AUTO_UPDATE_SIZE_LIMITS) + | B_NOT_RESIZABLE | B_AUTO_UPDATE_SIZE_LIMITS), + fRelease(NULL) { MoveTo(DataTranslationsSettings::Instance()->WindowCorner()); @@ -151,6 +152,10 @@ DataTranslationsWindow::_ShowConfigView(int32 id) fRightBox->RemoveChild(fConfigView); delete fConfigView; fConfigView = NULL; + if (fRelease != NULL) { + fRelease->Release(); + fRelease = NULL; + } } BMessage emptyMsg; @@ -165,6 +170,10 @@ DataTranslationsWindow::_ShowConfigView(int32 id) // force config views to all have the same color fRightBox->AddChild(fConfigView); + // Make sure the translator's image doesn't get unloaded while we are still + // showing a config view whose code is in the image + fRelease = roster->AcquireTranslator(id); + return B_OK; } @@ -176,6 +185,11 @@ DataTranslationsWindow::_ShowInfoView() fRightBox->RemoveChild(fConfigView); delete fConfigView; fConfigView = NULL; + if (fRelease != NULL) { + fRelease->Release(); + fRelease = NULL; + } + } BTextView* view = new BTextView("info text"); diff --git a/src/preferences/datatranslations/DataTranslationsWindow.h b/src/preferences/datatranslations/DataTranslationsWindow.h index 68e420750b..1e1f634e39 100644 --- a/src/preferences/datatranslations/DataTranslationsWindow.h +++ b/src/preferences/datatranslations/DataTranslationsWindow.h @@ -20,6 +20,7 @@ #include "TranslatorListView.h" +class BTranslatorReleaseDelegate; class DataTranslationsWindow : public BWindow { public: @@ -39,6 +40,7 @@ private: void _SetupViews(); TranslatorListView* fTranslatorListView; + BTranslatorReleaseDelegate* fRelease; BBox* fRightBox; BView* fConfigView;