# HG changeset patch
# User Benjamin Golinvaux <bgo@osimis.io>
# Date 1567438166 -7200
# Node ID 262a0244e9b2d38d50bec3494228da0ec71bf34f
# Parent  3abc47e051c882f3a35eeaf17f729ba9df5d3be2
Added missing Unregister for objects that register by the broker + logs + guard in FetchContext

diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Loaders/DicomStructureSetLoader.cpp
--- a/Framework/Loaders/DicomStructureSetLoader.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Loaders/DicomStructureSetLoader.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -204,10 +204,6 @@
              it = instances.begin(); it != instances.end(); ++it)
       {
         std::auto_ptr<OrthancRestApiCommand> command(new OrthancRestApiCommand);
-#if 0
-        if (logbgo115)
-          LOG(TRACE) << "  DicomStructureSetLoader::LoadStructure::Handle() about to schedule /tools/lookup command with LookupInstance on result";
-#endif
         command->SetUri("/tools/lookup");
         command->SetMethod(Orthanc::HttpMethod_Post);
         command->SetBody(*it);
@@ -217,21 +213,10 @@
         // They should NOT be required for POST requests
         //command->SetHttpHeader("pragma", "no-cache");
         //command->SetHttpHeader("cache-control", "no-cache");
-#if 0
-        std::string itStr(*it);
-        if(itStr == "1.3.12.2.1107.5.1.4.66930.30000018062412550879500002198") {
-          if (logbgo115)
-            LOG(ERROR) << "******** BOGUS LOOKUPS FROM NOW ON ***********";
-          logbgo233 = true;
-        }
-#endif
+
         command->SetPayload(new LookupInstance(loader, *it));
         //LOG(TRACE) << "About to schedule a /tools/lookup POST request. URI = " << command->GetUri() << " Body size = " << (*it).size() << " Body = " << (*it) << "\n";
         Schedule(command.release());
-#if 0
-        if (logbgo115)
-          LOG(TRACE) << "  DicomStructureSetLoader::LoadStructure::Handle() request scheduled. Command will be LookupInstance and post body is " << *it;
-#endif
       }
     }
   };
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Loaders/LoaderStateMachine.cpp
--- a/Framework/Loaders/LoaderStateMachine.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Loaders/LoaderStateMachine.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -136,10 +136,9 @@
   template <typename T>
   void LoaderStateMachine::HandleSuccessMessage(const T& message)
   {
-    LOG(TRACE) << "LoaderStateMachine(" << std::hex << this << std::dec << ")::HandleSuccessMessage()";
+    LOG(TRACE) << "LoaderStateMachine(" << std::hex << this << std::dec << ")::HandleSuccessMessage(). Receiver fingerprint = " << GetFingerprint();
     if (activeCommands_ <= 0) {
       LOG(ERROR) << "LoaderStateMachine(" << std::hex << this << std::dec << ")::HandleSuccessMessage : activeCommands_ should be > 0 but is: " << activeCommands_;
-      LOG(ERROR) << "LoaderStateMachine(" << std::hex << this << std::dec << ") fingerprint = " << GetFingerprint();
     }
     else {
       activeCommands_--;
@@ -162,6 +161,7 @@
                                          IObservable& oracleObservable) :
     IObserver(oracleObservable.GetBroker()),
     oracle_(oracle),
+    oracleObservable_(oracleObservable),
     active_(false),
     simultaneousDownloads_(4),
     activeCommands_(0)
@@ -187,6 +187,7 @@
 
   LoaderStateMachine::~LoaderStateMachine()
   {
+    oracleObservable_.Unregister(this);
     LOG(TRACE) << "LoaderStateMachine(" << std::hex << this << std::dec << ")::~LoaderStateMachine()";
     Clear();
   }
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Loaders/LoaderStateMachine.h
--- a/Framework/Loaders/LoaderStateMachine.h	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Loaders/LoaderStateMachine.h	Mon Sep 02 17:29:26 2019 +0200
@@ -95,6 +95,7 @@
     typedef std::list<IOracleCommand*>  PendingCommands;
 
     IOracle&         oracle_;
+    IObservable&     oracleObservable_;
     bool             active_;
     unsigned int     simultaneousDownloads_;
     PendingCommands  pendingCommands_;
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.cpp
--- a/Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -424,6 +424,7 @@
     IObserver(oracleObservable.GetBroker()),
     IObservable(oracleObservable.GetBroker()),
     oracle_(oracle),
+    oracleObservable_(oracleObservable),
     active_(false),
     simultaneousDownloads_(4),
     volume_(volume),
@@ -445,6 +446,7 @@
 
   OrthancSeriesVolumeProgressiveLoader::~OrthancSeriesVolumeProgressiveLoader()
   {
+    oracleObservable_.Unregister(this);
     LOG(TRACE) << "OrthancSeriesVolumeProgressiveLoader::~OrthancSeriesVolumeProgressiveLoader()";
   }
 
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.h
--- a/Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.h	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Loaders/OrthancSeriesVolumeProgressiveLoader.h	Mon Sep 02 17:29:26 2019 +0200
@@ -106,6 +106,7 @@
     void LoadJpegSliceContent(const GetOrthancWebViewerJpegCommand::SuccessMessage& message);
 
     IOracle&                                      oracle_;
+    IObservable&                                  oracleObservable_;
     bool                                          active_;
     unsigned int                                  simultaneousDownloads_;
     SeriesGeometry                                seriesGeometry_;
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Messages/ICallable.h
--- a/Framework/Messages/ICallable.h	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Messages/ICallable.h	Mon Sep 02 17:29:26 2019 +0200
@@ -78,13 +78,21 @@
 
     void ApplyInternal(const TMessage& message)
     {
-      if (observerFingerprint_ != observer_.GetFingerprint())
+      std::string currentFingerprint(observer_.GetFingerprint());
+      if (observerFingerprint_ != currentFingerprint)
       {
-        LOG(TRACE) << "The observer at address " << std::hex << &observer_ << std::dec << ") has a different fingerprint than the one recorded at callback registration time. This means that it is not the same object as the one recorded, even though their addresses are the same.  Callback will NOT be sent!";
-        LOG(TRACE) << " recorded fingerprint = " << observerFingerprint_ << " current fingerprint = " << observer_.GetFingerprint();
+        LOG(TRACE) << "The observer at address " << 
+          std::hex << &observer_ << std::dec << 
+          ") has a different fingerprint than the one recorded at callback " <<
+          "registration time. This means that it is not the same object as " <<
+          "the one recorded, even though their addresses are the same. " <<
+          "Callback will NOT be sent!";
+        LOG(TRACE) << " recorded fingerprint = " << observerFingerprint_ << 
+          " current fingerprint = " << currentFingerprint;
       }
       else
       {
+        LOG(TRACE) << "The recorded fingerprint is " << observerFingerprint_ << " and the current fingerprint is " << currentFingerprint << " -- callable will be called.";
         (observer_.*function_) (message);
       }
     }
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Messages/IObservable.cpp
--- a/Framework/Messages/IObservable.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Messages/IObservable.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -66,6 +66,8 @@
 
   void IObservable::Unregister(IObserver *observer)
   {
+    LOG(TRACE) << "IObservable::Unregister for IObserver at addr: "
+      << std::hex << observer << std::dec;
     // delete all callables from this observer
     for (Callables::iterator itCallableSet = callables_.begin();
          itCallableSet != callables_.end(); ++itCallableSet)
@@ -75,6 +77,8 @@
       {
         if ((*itCallable)->GetObserver() == observer)
         {
+          LOG(TRACE) << "  ** IObservable::Unregister : deleting callable: "
+            << std::hex << (*itCallable) << std::dec;
           delete *itCallable;
           itCallableSet->second.erase(itCallable++);
         }
@@ -87,6 +91,8 @@
   void IObservable::EmitMessageInternal(const IObserver* receiver,
                                         const IMessage& message)
   {
+    LOG(TRACE) << "IObservable::EmitMessageInternal receiver = "
+      << std::hex << receiver << std::dec;
     Callables::const_iterator found = callables_.find(message.GetIdentifier());
 
     if (found != callables_.end())
@@ -119,9 +125,10 @@
   void IObservable::EmitMessage(const IObserver& observer,
                                 const IMessage& message)
   {
+    LOG(TRACE) << "IObservable::EmitMessage observer = "
+      << std::hex << &observer << std::dec;
     EmitMessageInternal(&observer, message);
   }
-
   
   void IObservable::RegisterForwarder(IMessageForwarder* forwarder)
   {
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Messages/IObserver.h
--- a/Framework/Messages/IObserver.h	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Messages/IObserver.h	Mon Sep 02 17:29:26 2019 +0200
@@ -54,8 +54,33 @@
 
     virtual ~IObserver()
     {
-      LOG(TRACE) << "IObserver(" << std::hex << this << std::dec << ")::~IObserver : fingerprint_ == " << fingerprint_;
-      broker_.Unregister(*this);
+      try
+      {
+        LOG(TRACE) << "IObserver(" << std::hex << this << std::dec << ")::~IObserver : fingerprint_ == " << fingerprint_;
+        const char* deadMarker = "deadbeef-dead-dead-0000-0000deadbeef";
+        ORTHANC_ASSERT(strlen(deadMarker) == 36);
+        memcpy(fingerprint_, deadMarker, 37);
+        broker_.Unregister(*this);
+      }
+      catch (const Orthanc::OrthancException& e)
+      {
+        if (e.HasDetails())
+        {
+          LOG(ERROR) << "OrthancException in ~IObserver: " << e.What() << " Details: " << e.GetDetails();
+        }
+        else
+        {
+          LOG(ERROR) << "OrthancException in ~IObserver: " << e.What();
+        }
+      }
+      catch (const std::exception& e)
+      {
+        LOG(ERROR) << "std::exception in ~IObserver: " << e.what();
+      }
+      catch (...)
+      {
+        LOG(ERROR) << "Unknown exception in ~IObserver";
+      }
     }
 
     const char* GetFingerprint() const
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Oracle/WebAssemblyOracle.cpp
--- a/Framework/Oracle/WebAssemblyOracle.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Oracle/WebAssemblyOracle.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -89,11 +89,21 @@
     virtual void EmitMessage(const IObserver& receiver,
                              const IMessage& message)
     {
+      LOG(TRACE) << "WebAssemblyOracle::Emitter::EmitMessage receiver = "
+        << std::hex << &receiver << std::dec;
       oracle_.EmitMessage(receiver, message);
     }
   };
 
+  /**
+  This object is created on the heap for every http request.
+  It is deleted in the success (or error) callbacks.
 
+  This object references the receiver of the request. Since this is a raw
+  reference, we need additional checks to make sure we send the response to
+  the same object, for the object can be deleted and a new one recreated at the
+  same address (it often happens in the [single-threaded] browser context).
+  */
   class WebAssemblyOracle::FetchContext : public boost::noncopyable
   {
   private:
@@ -101,6 +111,7 @@
     const IObserver&               receiver_;
     std::auto_ptr<IOracleCommand>  command_;
     std::string                    expectedContentType_;
+    std::string                    receiverFingerprint_;
 
   public:
     FetchContext(WebAssemblyOracle& oracle,
@@ -110,8 +121,13 @@
       emitter_(oracle),
       receiver_(receiver),
       command_(command),
-      expectedContentType_(expectedContentType)
+      expectedContentType_(expectedContentType),
+      receiverFingerprint_(receiver.GetFingerprint())
     {
+      LOG(TRACE) << "WebAssemblyOracle::FetchContext::FetchContext() | "
+        << "receiver address = " << std::hex << &receiver << std::dec 
+        << " with fingerprint = " << receiverFingerprint_;
+
       if (command == NULL)
       {
         throw Orthanc::OrthancException(Orthanc::ErrorCode_NullPointer);
@@ -125,6 +141,8 @@
 
     void EmitMessage(const IMessage& message)
     {
+      LOG(TRACE) << "WebAssemblyOracle::FetchContext::EmitMessage receiver_ = "
+        << std::hex << &receiver_ << std::dec;
       emitter_.EmitMessage(receiver_, message);
     }
 
@@ -198,15 +216,38 @@
 
       // an UUID is 36 chars : 32 hex chars + 4 hyphens: char #0 --> char #35
       // char #36 is \0.
+      bool callHandler = true;
       
       // TODO: remove this line because we are NOT allowed to call methods on GetReceiver that is maybe a dangling ref
-      if (context->GetReceiver().DoesFingerprintLookGood()) {
-        LOG(TRACE) << "SuccessCallback for object at address (" << std::hex << &(context->GetReceiver()) << std::dec << " with current fingerprint = " << context->GetReceiver().GetFingerprint();
+      if (context->GetReceiver().DoesFingerprintLookGood())
+      {
+        callHandler = true;
+        std::string currentFingerprint(context->GetReceiver().GetFingerprint());
+       
+        LOG(TRACE) << "SuccessCallback for object at address (" << std::hex 
+          << &(context->GetReceiver()) << std::dec 
+          << " with current fingerprint = " << currentFingerprint 
+          << ". Fingerprint looks OK";
+        
+        if (currentFingerprint != context->receiverFingerprint_)
+        {
+          LOG(TRACE) << "  ** SuccessCallback: BUT currentFingerprint != "
+            << "receiverFingerprint_(" << context->receiverFingerprint_ << ")";
+          callHandler = false;
+        }
+        else
+        {
+          LOG(TRACE) << "  ** SuccessCallback: FetchContext-level "
+            << "fingerprints are the same: "
+            << context->receiverFingerprint_
+            << " ---> oracle will dispatch the response to observer: "
+            << std::hex << &(context->GetReceiver()) << std::dec;
+        }
       }
       else {
         LOG(TRACE) << "SuccessCallback for object at address (" << std::hex << &(context->GetReceiver()) << std::dec << " with current fingerprint is XXXXX -- NOT A VALID FINGERPRINT! OBJECT IS READ ! CALLBACK WILL NOT BE CALLED!";
+        callHandler = false;
       }
-      
 
       if (fetch->userData == NULL)
       {
@@ -261,11 +302,13 @@
         }
         else
         {
-          if (context->GetReceiver().DoesFingerprintLookGood()) {
+          if (callHandler)
+          {
             switch (context->GetCommand().GetType())
             {
             case IOracleCommand::Type_OrthancRestApi:
             {
+              LOG(TRACE) << "WebAssemblyOracle::FetchContext::SuccessCallback. About to call context->EmitMessage(message);";
               OrthancRestApiCommand::SuccessMessage message
               (context->GetTypedCommand<OrthancRestApiCommand>(), headers, answer);
               context->EmitMessage(message);
@@ -611,13 +654,10 @@
   void WebAssemblyOracle::Schedule(const IObserver& receiver,
                                    IOracleCommand* command)
   {
-#if 0
-    if (logbgo233) {
-      if (logbgo115)
-        LOG(TRACE) << "      WebAssemblyOracle::Schedule command addr " <<
-        std::hex << command << std::dec;
-    }
-#endif
+    LOG(TRACE) << "WebAssemblyOracle::Schedule : receiver = "
+      << std::hex << &receiver << std::dec
+      << " | Current fingerprint is " << receiver.GetFingerprint();
+    
     std::auto_ptr<IOracleCommand> protection(command);
 
     if (command == NULL)
diff -r 3abc47e051c8 -r 262a0244e9b2 Framework/Scene2DViewport/MeasureTool.cpp
--- a/Framework/Scene2DViewport/MeasureTool.cpp	Sat Aug 31 13:50:11 2019 +0200
+++ b/Framework/Scene2DViewport/MeasureTool.cpp	Mon Sep 02 17:29:26 2019 +0200
@@ -30,7 +30,7 @@
 {
   MeasureTool::~MeasureTool()
   {
-
+    GetController()->Unregister(this);
   }
 
   void MeasureTool::Enable()