Pk, 2011-10-14 00:04 +0300, My Th rakstīja:
> C , 2011-10-13 23:13 +0300, My Th rakstīja:
> > C , 2011-10-13 13:53 -0400, David Lonie rakstīja:
> > > There does seem to be a new problem that took its place. In the
> > > attached script, I've added line 36, which will print the name of the
> > > forcefield that was found. When I run the test script now, I get:
> > > 
> > > Forcefield is of type: UFF
> > > Forcefield is of type: GAFF
> > > 
> > > despite using FindForceField("UFF") in each case. So something is
> > > still not quite there. Any ideas?
> > 
> > Yes, I misunderstood the function of Default() a bit. Turns out that it
> > is a pointer to an instance of default sub-type for a plugin type (not
> > to the default instance of a sub-type).
> > 
> > As I understand, plugins do not keep track of their instances. So when
> > you create new one with MakeNewInstance() the reference to old one is
> > lost (Doesn't it lead to memory leaks?).
> > 
> > So if we add a static member variable to the OBForceFieldUFF class which
> > keeps pointers to the live instances, then we can use that in destructor
> > to reset plugin map pointers to the original instance. Something like in
> > the attached patch for OBForceFieldUFF.
> > 
> > I'm not sure if this is the best solution, but it seems like something
> > that is working. Hope it helps.
> 
> Other solution would be to change the constructor so that pointers in
> plugin map are not overwritten if the instance with the same ID already
> exists.
> 
> That basically leaves open the same question I made yesterday - what to
> do when the last instance of the plugin sub-type is destroyed?

Here is a patch for plugin constructors. It is a bit big because I
decompacted the macros to make them readable, but there is really only
one change - an "if" statement which checks if the ID is already in the
Map() and only if not then it assigns "this" to Map() and PluginMap().

With this patch there is no need for any of the previous patches.

I'm not sure if anyone has relied on the availability of the new
instances in plugin map, but AFAIK this is the simplest way to make
MakeNewInstance() work and be thread safe.

I'm curious to hear opinions from others :)


Reinis
Index: include/openbabel/plugin.h
===================================================================
--- include/openbabel/plugin.h	(revision 4642)
+++ include/openbabel/plugin.h	(working copy)
@@ -148,36 +148,75 @@
 //Macro to be added to definition of the base class
 #define MAKE_PLUGIN(BaseClass)\
 protected:\
-static PluginMapType& Map();\
-virtual PluginMapType& GetMap()const{return Map();}\
+  static PluginMapType& Map();\
+  virtual PluginMapType& GetMap() const {\
+    return Map();\
+  }\
 public:\
-static BaseClass*& Default(){static BaseClass* d;return d;}\
-  BaseClass(const char* ID, bool IsDefault=false)\
- {_id=ID;if(ID&&*ID){if(IsDefault || Map().empty()) Default() = this;\
- Map()[ID]=this;PluginMap()[TypeID()] =this;}}\
-static BaseClass* FindType(const char* ID)\
- {if(!ID || *ID==0 || *ID==' ') return Default();\
- return static_cast<BaseClass*>(BaseFindType(Map(),ID));}
+  static BaseClass*& Default() {\
+    static BaseClass* d;\
+    return d;\
+  }\
+  BaseClass(const char* ID, bool IsDefault=false) {\
+    _id=ID;\
+    if (ID&&*ID) {\
+      if (IsDefault || Map().empty()) {\
+        Default() = this;\
+      }\
+      if (Map().count(ID) == 0) {\
+        Map()[ID] = this;\
+        PluginMap()[TypeID()] = this;\
+      }\
+    }\
+  }\
+  static BaseClass* FindType(const char* ID) {\
+    if (!ID || *ID==0 || *ID==' ') {\
+      return Default();\
+    }\
+    return static_cast<BaseClass*>(BaseFindType(Map(),ID));\
+  }
 
 #define PLUGIN_CPP_FILE(BaseClass)\
-OBPlugin::PluginMapType& BaseClass::Map()\
-{ static OBPlugin::PluginMapType map; return map; }
+OBPlugin::PluginMapType& BaseClass::Map() {\
+  static OBPlugin::PluginMapType map;\
+  return map;\
+}
 
 #else // __CYGWIN__ || __MINGW32__
 
 //Macro to be added to definition of the base class
 #define MAKE_PLUGIN(BaseClass)\
 protected:\
-static PluginMapType& Map(){static PluginMapType m;return m;}\
-virtual PluginMapType& GetMap()const{return Map();}\
+  static PluginMapType& Map() {\
+    static PluginMapType m;\
+    return m;\
+  }\
+  virtual PluginMapType& GetMap() const {\
+    return Map();\
+  }\
 public:\
-static BaseClass*& Default(){static BaseClass* d;return d;}\
-  BaseClass(const char* ID, bool IsDefault=false)\
- {_id=ID;if(ID&&*ID){if(IsDefault || Map().empty()) Default() = this;\
- Map()[ID]=this;PluginMap()[TypeID()] =this;}}\
-static BaseClass* FindType(const char* ID)\
- {if(!ID || *ID==0 || *ID==' ') return Default();\
- return static_cast<BaseClass*>(BaseFindType(Map(),ID));}
+  static BaseClass*& Default() {\
+    static BaseClass* d;\
+    return d;\
+  }\
+  BaseClass(const char* ID, bool IsDefault=false) {\
+    _id=ID;\
+    if (ID&&*ID) {\
+      if (IsDefault || Map().empty()) {\
+        Default() = this;\
+      }\
+      if (Map().count(ID) == 0) {\
+        Map()[ID] = this;\
+        PluginMap()[TypeID()] = this;\
+      }\
+    }\
+  }\
+  static BaseClass* FindType(const char* ID) {\
+    if (!ID || *ID==0 || *ID==' ') {\
+      return Default();\
+    }\
+    return static_cast<BaseClass*>(BaseFindType(Map(),ID));\
+  }
 
 #endif // __CYGWIN__ || __MINGW32__
 
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2d-oct
_______________________________________________
OpenBabel-Devel mailing list
OpenBabel-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openbabel-devel

Reply via email to