Memory Leak in Array<T> class

[John W. Eaton]

On 30-Oct-2004, David Bateman <address@hidden> wrote:

| All of the void constructors derived from Array<T> have a memory leak. The
| bit of the class that describes the problem is
| 
| template <class T>
| class
| Array
| {
| protected:
|   class ArrayRep
|   {
|   public:
|     ArrayRep (void) : data (0), len (0), count (1) { }
|   };
| private:
| 
|   typename Array<T>::ArrayRep *nil_rep (void) const
|     {
|       static typename Array<T>::ArrayRep *nr
|       = new typename Array<T>::ArrayRep ();
| 
|       return nr;
|     }
| 
|   Array (void)
|     : rep (nil_rep ()), dimensions (),
|       idx (0), idx_count (0) { rep->count++; }
| };
| 
| So what happens is that the void constructor calls nil_rep which creates
| the internal representation and adds one to the count. A new ArrayRep
| is created for each void constructor. However the constructor itself
| then adds 1 to rep->count and the ArrayRep therefore becomes immortal
| as its count can never reach 1, when the Array<T>::~Array destructor
| is called. This can easily be demostrated with the code snippet.
| 
| #include <octave/config.h>
| #include <octave/oct.h>
| 
| DEFUN_DLD (foo, args, , " ")
| {
|   NDArray a;
| 
|   a.print_info (octave_stdout, " ");
|   return octave_value ();
| }
| 
| which returns
| 
| octave:1> foo
|  rep address: 0x8a4fee8
|  rep->len:    0
|  rep->data:   0
|  rep->count:  2

If this code caused a memory leak, then running the above in a loop
like this:

  page_screen_output = 0;
  while (1)
    foo;
  endwhile

or, to speed things up a bit, modifying it to be

  #include <octave/config.h>
  #include <octave/oct.h>

  DEFUN_DLD (xxx, args, , " ")
  {
    NDArray a;
    return octave_value ();
  }

and running the same loop would cause Octave to grow without bound
(even if the growth was slow).  But that is not what I see.

The key is the definition of the nil_rep function:

  typename Array<T>::ArrayRep *nil_rep (void) const
    {
      static typename Array<T>::ArrayRep *nr
        = new typename Array<T>::ArrayRep ();

      return nr;
    }

Note that the returned value is static within the function, so it is
constructed just once, when NR is initialized.  Each time nil_rep is
called, we return the same empty ArrayRep object.  It is correct to
increment the reference count for this object in the constructors,
because we never want the static NR object to be destroyed.  Defining
nil_rep this way is an attempt to speed things up by avoiding
unnecessary calls to new and delete to repeatedly create an empty
ArrayRep object.  Since all empty ArrayRep objects have the same
representation, we should only have to create it once.

jwe