0025057: Message_Algorithm fails to find messages defined for base class

New method HasMsg() is added in Message_MsgFile class to check if message with given key is registered.
That method is used in Message_Algorithm to check if message is defined on given level of class hierarchy.

Generation of error message in Message_MsgFile::Msg() is revised: now it includes a message key and gets added to the registry, to avoid re-generation in case of multiple requests.
Access to message registry maintained by the Message_MsgFile class is protected from concurrent access by mutex.

diff --git a/src/Message/Message_Algorithm.cxx b/src/Message/Message_Algorithm.cxx
index faa72db..28f8c74 100644 (file)
--- a/src/Message/Message_Algorithm.cxx
+++ b/src/Message/Message_Algorithm.cxx
@@ -224,7 +224,7 @@ void Message_Algorithm::SendStatusMessages (const Message_ExecStatus& theStatus,
     // find message, iterating by base classes if necessary
     TCollection_AsciiString aMsgName = aClassName + aSuffix;
     Handle(Standard_Type) aType = DynamicType();
-    while (Message_MsgFile::Msg(aMsgName).Length() == 0 && !aType.IsNull())
+    while (! Message_MsgFile::HasMsg(aMsgName) && !aType.IsNull())
     {
       Standard_AncestorIterator it(aType);
       aType.Nullify();
@@ -233,7 +233,7 @@ void Message_Algorithm::SendStatusMessages (const Message_ExecStatus& theStatus,
         aType = it.Value();
         TCollection_AsciiString aClassName1 (aType->Name());
         TCollection_AsciiString aMsgName1 = aClassName1 + aSuffix;
-        if (Message_MsgFile::Msg(aMsgName1).Length() != 0)
+        if (Message_MsgFile::HasMsg(aMsgName1))
         {
           aMsgName = aMsgName1;
           break;

diff --git a/src/Message/Message_MsgFile.cdl b/src/Message/Message_MsgFile.cdl
index cca9182..770aec7 100644 (file)
--- a/src/Message/Message_MsgFile.cdl
+++ b/src/Message/Message_MsgFile.cdl
@@ -74,6 +74,9 @@ is
         --          If there already was defined the message identified by the 
         --          same keyword, it is replaced with the new one.
 
+    HasMsg (myclass; key: AsciiString from TCollection) returns Boolean;
+        ---Purpose: Returns True if message with specified keyword is registered
+
     Msg (myclass; key: CString) returns ExtendedString from TCollection;
        ---C++: return const &
     Msg (myclass; key: AsciiString from TCollection) returns ExtendedString from TCollection;

diff --git a/src/Message/Message_MsgFile.cxx b/src/Message/Message_MsgFile.cxx
index 502fc14..9d4c671 100644 (file)
--- a/src/Message/Message_MsgFile.cxx
+++ b/src/Message/Message_MsgFile.cxx
@@ -19,6 +19,7 @@
 #include <OSD_Environment.hxx>
 #include <TCollection_AsciiString.hxx>
 #include <TCollection_ExtendedString.hxx>
+#include <Standard_Mutex.hxx>
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -31,6 +32,9 @@ static Message_DataMapOfExtendedString& msgsDataMap ()
   return aDataMap;
 }
 
+// mutex used to prevent concurrent access to message registry
+static Standard_Mutex theMutex;
+
 typedef enum
 {
   MsgFile_WaitingKeyword,
@@ -310,8 +314,8 @@ Standard_Boolean Message_MsgFile::AddMsg (const TCollection_AsciiString& theKeyw
                                          const TCollection_ExtendedString&  theMessage)
 {
   Message_DataMapOfExtendedString& aDataMap = ::msgsDataMap();
-//  if (aDataMap.IsBound (theKeyword))
-//    return Standard_False;
+
+  Standard_Mutex::Sentry aSentry (theMutex);
   aDataMap.Bind (theKeyword, theMessage);
   return Standard_True;
 }
@@ -328,7 +332,18 @@ const TCollection_ExtendedString &Message_MsgFile::Msg (const Standard_CString t
 } 
 
 //=======================================================================
-//function : getMsg
+//function : HasMsg
+//purpose  : 
+//=======================================================================
+
+Standard_Boolean Message_MsgFile::HasMsg (const TCollection_AsciiString& theKeyword)
+{
+  Standard_Mutex::Sentry aSentry (theMutex);
+  return ::msgsDataMap().IsBound (theKeyword);
+}
+
+//=======================================================================
+//function : Msg
 //purpose  : retrieve the message previously defined for the given keyword
 //=======================================================================
 
@@ -336,23 +351,19 @@ const TCollection_ExtendedString &Message_MsgFile::Msg (const TCollection_AsciiS
 {
   // find message in the map
   Message_DataMapOfExtendedString& aDataMap = ::msgsDataMap();
-  if (aDataMap.IsBound (theKeyword))
-    return aDataMap.Find (theKeyword);
-
-  // if not found, generate error message
-  // to minimize risk of data races when running concurrently, set the static variables
-  // only if they are empty; this gives a possibility to enforce calling this method
-  // upfront to initialize these variables and only read access them afterwards. However
-  // theKeyword is no longer appended. aDefPrefix remained unchanged to not break some
-  // logs which might expect the previous value
-  static const TCollection_ExtendedString aDefPrefix ("Unknown message invoked with the keyword");
-  static const TCollection_AsciiString aPrefixCode ("Message_Msg_BadKeyword");
-  static TCollection_ExtendedString aFailureMessage;
-  if (aFailureMessage.Length() == 0) {
-     if (aDataMap.IsBound (aPrefixCode))
-       aFailureMessage = aDataMap.Find (aPrefixCode);
-     else
-       aFailureMessage = aDefPrefix;
+  Standard_Mutex::Sentry aSentry (theMutex);
+
+  // if message is not found, generate error message and add it to the map to minimize overhead
+  // on consequent calls with the same key
+  if (! aDataMap.IsBound(theKeyword))
+  {
+    // text of the error message can be itself defined in the map
+    static const TCollection_AsciiString aPrefixCode("Message_Msg_BadKeyword");
+    static const TCollection_ExtendedString aDefPrefix("Unknown message invoked with the keyword ");
+    TCollection_AsciiString aErrorMessage = (aDataMap.IsBound(aPrefixCode) ? aDataMap(aPrefixCode) : aDefPrefix);
+    aErrorMessage += theKeyword;
+    aDataMap.Bind (theKeyword, aErrorMessage); // do not use AddMsg() here to avoid mutex deadlock
   }
-  return aFailureMessage;
+
+  return aDataMap (theKeyword);
 }