Ticket #12005: 0002-Make-sure-images-containing-BTranslators-are-not-unl.2.patch

headers/os/app/AppDefs.h

@@ -59 +59 @@
     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',

headers/os/translation/TranslatorRoster.h

@@ -23 +23 @@
 struct entry_ref;
 
 
+class BTranslatorReleaseDelegate {
+public:
+                                BTranslatorReleaseDelegate(BTranslator* translator);
+
+            void                Release();
+
+private:
+            BTranslator*        fUnderlying;
+};
+
+
 class BTranslatorRoster : public BArchivable {
 public:
                                 BTranslatorRoster();
@@ -86 +97 @@
                                     translator_id translatorID,
                                     BMessage* ioExtension);
 
+            BTranslatorReleaseDelegate* AcquireTranslator(int32 translatorID);
+
             status_t            GetRefFor(translator_id translatorID,
                                     entry_ref* ref);
             bool                IsTranslator(entry_ref* ref);

src/kits/translation/Translator.cpp

@@ -24 +24 @@
 
 BTranslator::~BTranslator()
 {
-    if (fOwningRoster != NULL)
-        fOwningRoster->TranslatorDeleted(fID);
 }
 
 
@@ -52 +50 @@
 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;
 }
 

src/kits/translation/TranslatorRoster.cpp

@@ -193 +193 @@
     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 +315 @@
             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 +602 @@
             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 +611 @@
             }
         }
 
-        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 +865 @@
 
 
 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 +1097 @@
         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 +1107 @@
         iterator = next;
     }
 
-    // Unload image from the removed translator
-
-    if (image >= B_OK)
-        unload_add_on(image);
-
     _NotifyListeners(update);
 }
 
@@ -1150 +1166 @@
 
 //  #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 +1696 @@
 }
 
 
+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.

src/kits/translation/TranslatorRosterPrivate.h

@@ -33 +33 @@
 typedef std::vector<BMessenger> MessengerList;
 typedef std::vector<node_ref> NodeRefList;
 typedef std::set<entry_ref> EntryRefSet;
+typedef std::map<image_id, int32> ImageMap;
+typedef std::map<BTranslator*, image_id> TranslatorImageMap;
 
 
 class BTranslatorRoster::Private : public BHandler, public BLocker {
@@ -78 +80 @@
             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 +107 @@
                                     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;

src/preferences/datatranslations/DataTranslationsWindow.cpp

@@ -53 +53 @@
     :
     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 +152 @@
         fRightBox->RemoveChild(fConfigView);
         delete fConfigView;
         fConfigView = NULL;
+        if (fRelease != NULL) {
+            fRelease->Release();
+            fRelease = NULL;
+        }
     }
 
     BMessage emptyMsg;
@@ -165 +170 @@
         // 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 +185 @@
         fRightBox->RemoveChild(fConfigView);
         delete fConfigView;
         fConfigView = NULL;
+        if (fRelease != NULL) {
+            fRelease->Release();
+            fRelease = NULL;
+        }
+
     }
 
     BTextView* view = new BTextView("info text");

src/preferences/datatranslations/DataTranslationsWindow.h

@@ -20 +20 @@
 
 #include "TranslatorListView.h"
 
+class BTranslatorReleaseDelegate;
 
 class DataTranslationsWindow : public BWindow {
 public:
@@ -39 +40 @@
             void            _SetupViews();
 
             TranslatorListView* fTranslatorListView;
+            BTranslatorReleaseDelegate*     fRelease;
 
             BBox*           fRightBox;
             BView*          fConfigView;