memory question

[Severin Ecker-2]

Hi again,

I've found a few interesting spots in the code and now i have a question
regarding the memory allocation schemes of cint:
does cint come with it's own memory management and if so when and where
is it used? (is it used within the library itself or only for the
interpreter environment)

e.g.: take these few lines from Namespace.cxx

const Reflex::Scope & Reflex::Namespace::GlobalScope() {
   static Scope s = (new Namespace())->ThisScope();
   return s;
}

If we're not in a memory managed environment, this code will leak
(unless the namespace object is extracted from the scope object
somewhere else and then deleted in a suicide kind of way...).
My question is: is this a leak, or is it intentional because it
is/should be cleaned up through some management magic (this code is
called from G__scratch_all() (G__scratch_upto_work()), so I suppose it's
code run within the C-runtime and not within the cint interpreter, right?

cheers,
severin

[Severin Ecker-2]

Hi,

I've been looking deeply and long into the code and I have a more or
less complete list of the memory leaks that exist in reflex and cint7

before i come to that, there's also another path_patch attached with a
few things that I've missed the first time.

right:

let's start with cint7 (for this there's a patch attached)

-) init.cxx line 146  (I've added a G__cleanup_cint function to the cint
API which destroys the list, since i haven't found an appropriate
function where I could have safely deleted the list without breaking any
other code)

now for the reflex lib

one leak I've fixed he others are really hard without knowing the
internals of who owns what and is allowed to free memory, I've shot my
leg a few times while trying to fix these (a lot of those look horribly
like java code!!!).

fixed:
-) namespace.cxx line 41 (simply replaced the new Namespace() with
retrieving a statically allocated stack object from another function)

not fixed:

-) ClassBuilder.cxx line 68 (fClass leaks, I think i can safely delete
it in the destructor but i have to recheck this again)

-) FunctionBuilder.cxx line 95 (Namespace object leaks)
                                   line 160 (Namespace object leaks)
                                   line 166 (FunctionMember object leaks)
                                   line 169 (FunctionMember object leaks)

-) Kernel.cxx lines 57 and following (all Fundamental and Typedef
objects leak)

-) NamespaceBuilder.cxx line 37 (Namespace object leaks)

-) Scope.cxx line 32 (ScopeName object leaks)

-) ScopeBase.cxx line 68 (ScopeName object leaks)
                            line 77 (Namespace object leaks)
                            line 78 (ScopeName object leaks)
                            line 103 (ScopeName object leaks)
                            line 638 (DataMember object leaks)

-) ScopeName.cxx line 58 (Scope object leaks)
                              line 64 (ScopeName object leaks)

-) TypeBase.cxx line 62 (TypeName object leaks)
                          line 76 (ScopeName object leaks)
                          line 436 (Type object _seems_ to leak)

-) TypeBuilder.cxx line 40 (ScopeName object leaks)
                             line 72 (Pointer object leaks)
                             line 84 (PointerToMember object leaks)
                             line 155 (Typedef object leaks)
                             line 165, 176, 188, 201, 215, 230, 246,
263, 281, 300, 320, 341, 363, 386, 410, 435, 461, 488, 517, 547, 578,
610, 643, 677, 712, 748, 785, 823, 862, 902, 943, 985, 1029, 1074
(Function objects leak)

-) TypeName.cxx line 66 (Type object leaks)

-) VariableBuilder.cxx line 43 (Namespace object leaks)
                                  line 103 (Namespace object leaks)

There's also some problem with the PropertyListImpl but I haven't yet
nailed the problem down.

Alright, that's it what I've found so far. Can you give me some details
of who owns what and who is allowed/obliged to cleanup after itself?
Also, have you considered using some kind of registry for all those
objects (some seem to be not used anyway.. (new xx)->GetY() so they get
properly deleted after shutting down? I'm working on something like this
but still have problems with some objects getting deleted more than once
so an ownership info/graph would really be helpful and time saving)

That's it for now.
cheers,
severin


Index: cint7/inc/G__ci_fproto.h
===================================================================
--- cint7/inc/G__ci_fproto.h (revision 27152)
+++ cint7/inc/G__ci_fproto.h (working copy)
@@ -211,6 +211,7 @@
 G__DECL_API(117, int , G__unloadfile, (G__CONST char* filename));
 G__DECL_API(258, int , G__setfilecontext, (G__CONST char* filename, struct G__input_file* ifile));
 G__DECL_API(118, int, G__init_cint, (G__CONST char* command));
+G__DECL_API(262, void, G__cleanup_cint, (void));
 G__DECL_API(119, void, G__scratch_all, (void));
 G__DECL_API(120, void, G__setdouble, (G__value *pbuf,double d,void* pd,int type,int tagnum,int typenum,int reftype));
 G__DECL_API(121, void, G__setint, (G__value *pbuf,long d,void* pd,int type,int tagnum,int typenum,int reftype));
@@ -385,5 +386,5 @@
 #endif
 G__DECL_API(256, void, G__letbool, (G__value* buf,int type,long value));
 
-#define G__NUMBER_OF_API_FUNCTIONS 262
+#define G__NUMBER_OF_API_FUNCTIONS 263
 G__DUMMYTOCHECKFORDUPLICATES(G__NUMBER_OF_API_FUNCTIONS)
Index: cint7/src/init.cxx
===================================================================
--- cint7/src/init.cxx (revision 27152)
+++ cint7/src/init.cxx (working copy)
@@ -130,6 +130,12 @@
       }
 }
 
+extern "C" void G__cleanup_cint(void)
+{
+ delete G__initpermanentsl;
+ G__initpermanentsl = 0;
+}
+
 //______________________________________________________________________________
 extern "C" int G__call_setup_funcs()
 {
Index: reflex/src/Namespace.cxx
===================================================================
--- reflex/src/Namespace.cxx (revision 27152)
+++ reflex/src/Namespace.cxx (working copy)
@@ -38,7 +38,7 @@
 const Reflex::Scope & Reflex::Namespace::GlobalScope() {
 //-------------------------------------------------------------------------------
 // Initialise the global namespace at startup.
-   static Scope s = (new Namespace())->ThisScope();
+   static Scope s = Namespace::GetNamespace().ThisScope();
    return s;
 }
 
@@ -74,3 +74,10 @@
   
 }
 
+
+//-------------------------------------------------------------------------------
+const Reflex::Namespace & Reflex::Namespace::GetNamespace(void) {
+//-------------------------------------------------------------------------------
+   static Namespace ns;
+   return ns;
+}
Index: reflex/src/Namespace.h
===================================================================
--- reflex/src/Namespace.h (revision 27152)
+++ reflex/src/Namespace.h (working copy)
@@ -54,10 +54,10 @@
       static const Scope & GlobalScope();
 
    private:
+  static const Namespace& GetNamespace(void);
 
       /** constructor for initialisation of the global namespace */
       Namespace();
-
    }; // class Namespace
 } //namespace Reflex
 

Index: cint7/src/loadfile.cxx
===================================================================
--- cint7/src/loadfile.cxx (revision 27152)
+++ cint7/src/loadfile.cxx (working copy)
@@ -1271,8 +1271,8 @@
        * try $CINTSYSDIR/stl
        **********************************************/
       if('\0'!=G__cintsysdir[0]) {
-         sprintf(workname,"%s/%s/stl/%s%s",G__cintsysdir,G__CFG_COREVERSION
-                 ,filename,addpost[i2]);
+         sprintf(workname,"%s%s%s%sstl%s%s%s",G__cintsysdir,G__psep,G__CFG_COREVERSION
+                 ,G__psep,G__psep,filename,addpost[i2]);
          res = stat( workname, statBuf );        
          if (res==0) return res;
       }
@@ -1282,8 +1282,8 @@
        **********************************************/
       /* G__getcintsysdir(); */
       if('\0'!=G__cintsysdir[0]) {
-         sprintf(workname,"%s/%s/lib/%s%s",G__cintsysdir,G__CFG_COREVERSION
-                 ,filename,addpost[i2]);
+         sprintf(workname,"%s%s%s%slib%s%s%s",G__cintsysdir,G__psep,G__CFG_COREVERSION
+                 ,G__psep,G__psep,filename,addpost[i2]);
          res = stat( workname, statBuf );        
          if (res==0) return res;
       }

[Severin Ecker-2]

Hi,

Here's a sample for the const_iterator 2-param problem.
As a short recap:

prec_stl/vector contains the following lines:

#if (G__MSC_VER>=1310) && (G__MSC_VER<1400)
        //const_iterator(pointer _Ptr);
        const_iterator(T *const  _Ptr);
#elif (G__MSC_VER>=1400)
        const_iterator(T *const  _Ptr, const vector<T>* _Pvector );
#endif

this does only work for debug builds on MSVC8. VC8's vector checks
whether _HAS_ITERATOR_DEBUGGING=1 and only then there's a 2 parameter
constructor for const_iterator available. This macro though is a debug
feature and i very much would like to disable it, but then the code
attached doesn't work anymore.

I've tried to just change the check macro above to G__MSC_VER < 1500 and
G__MSC_VER>=1500, but then i get segmentation faults when i try to run
the resulting cint executable so there seems to be a more serious problem.
I'd appreciate any help or a pointer to what might go wrong if
_HAS_ITERATOR_DEBUGGING=0.

thanks and cheers,
severin

ps.: attached are the simple header file which I used to create an
interpreter from. also attached are the cpp and h file that cint
produced. note at line 457 there's the call to the 2 parameter version
of the const_iterator constructor.