Memory leak - What the ...?

Memory leak - What the ...?

[Alex R. Mosteo]

Hi, 

as the topic says, this post is about some code which leaks. I'm now
sure of having trapped the leak, but I don't understand where is my
error.

The culprit is a controlled type, one of whose members is an access to
a Stream_Element_Array. The idea is to have an array of the exact
length required and not a worst case one, without using an
unconstrained type.

Platform is gnat 3.15p, compiling with assertions enabled (-gnata),
either win32 or linux.

Running a lot of tests with gnatmem, it always shows the allocation
not deallocated being at:

s-assert.adb:46                        
system.assertions.raise_assert_failure
adagio-g2-transceiver_types.adb:134    
adagio.g2.transceiver_types.adjust
<other stack calls not meaningful>

I don't know what the assertion means there, since I never get any
exception anyway.

The type declaration is:

   type Udp_message is new Finalization.Controlled with record
      Data : Stream_Element_Array_Access;
      Dest : Gnat.Sockets.Sock_addr_type;
      Date : Calendar.Time;
   end record;

The adjust procedure is:

   procedure Adjust (This : in out Udp_Message) is
   begin
      if This.Data /= null then
         This.Data := new Stream_Element_Array'(This.Data.all);
      end if;
   end Adjust;

And the finalize one is:

   procedure Finalize (This : in out Udp_Message) is
   begin
      Free (This.Data);
   end Finalize;

Now, where it gets interesting is that I've double and triple checked
that no objects of Udp_Message type are being leaked. Using counters
and traces I've certified that every object of that type which is
created is eventually finalized. No code mangles with the Data
pointer, so there's no outside manipulation which could cause the
leak.

Is there something flawled in this implementation? I just can't figure
why there is a leak. Changing the allocated array to a static array
removes the leak.

These objects are stored in an Ada.Containers.Doubly_Linked_List, if
that could add something.

Thanks in advance,

A. Mosteo.

Re: Memory leak - What the ...?

[Marius Amado Alves]

>       if This.Data /= null then
>          This.Data := new Stream_Element_Array'(This.Data.all);
>       end if;

To my pointerless eyes this looks very leaky.

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Marius Amado Alves <amado.alves@netcabo.pt> wrote in message news:<mailman.267.1097445952.390.comp.lang.ada@ada-france.org>...
> >       if This.Data /= null then
> >          This.Data := new Stream_Element_Array'(This.Data.all);
> >       end if;
> 
> To my pointerless eyes this looks very leaky.

Could you elaborate? I'm trying to do a deep copy.

Re: Memory leak - What the ...?

[Marius Amado Alves]

Alex R. Mosteo wrote:
> Marius Amado Alves <amado.alves@netcabo.pt> wrote in message news:<mailman.267.1097445952.390.comp.lang.ada@ada-france.org>...
> 
>>>      if This.Data /= null then
>>>         This.Data := new Stream_Element_Array'(This.Data.all);
>>>      end if;
>>
>>To my pointerless eyes this looks very leaky.
> 
> Could you elaborate? I'm trying to do a deep copy.

The "new" expression yields a new pointer value, the assignment smashes
the old one with this new value, so the old pointer is lost, and unless
some magic was put it place the old pointee becomes a zombie.

Re: Memory leak - What the ...?

[Martin Krischik]

Marius Amado Alves wrote:

> Alex R. Mosteo wrote:
>> Marius Amado Alves <amado.alves@netcabo.pt> wrote in message
>> news:<mailman.267.1097445952.390.comp.lang.ada@ada-france.org>...
>> 
>>>>      if This.Data /= null then
>>>>         This.Data := new Stream_Element_Array'(This.Data.all);
>>>>      end if;
>>>
>>>To my pointerless eyes this looks very leaky.
>> 
>> Could you elaborate? I'm trying to do a deep copy.
> 
> The "new" expression yields a new pointer value, the assignment smashes
> the old one with this new value, so the old pointer is lost, and unless
> some magic was put it place the old pointee becomes a zombie.

Well there is allways the shallow part of the deep copy. The original
pointer is still part of the original record. When the original record is
destroyed a deep delete (in Finalise) should deal with the original
pointer.

With Regards

Martin

-- 
mailto://krischik@users.sourceforge.net
http://www.ada.krischik.com

Re: Memory leak - What the ...?

[Martin Krischik]

Marius Amado Alves wrote:

>>     if This.Data /= null then
>>          This.Data := new Stream_Element_Array'(This.Data.all);
>>       end if;
> 
> To my pointerless eyes this looks very leaky.

Why? It look OK to me. Of course one would expect This.Data to be
initialised in Initialize so that This.Data is allways /= null...

With Regards

Martin
-- 
mailto://krischik@users.sourceforge.net
http://www.ada.krischik.com

Re: Memory leak - What the ...?

[Martin Dowie]

Martin Krischik wrote:
> Marius Amado Alves wrote:
>
>>>     if This.Data /= null then
>>>          This.Data := new Stream_Element_Array'(This.Data.all);
>>>       end if;
>>
>> To my pointerless eyes this looks very leaky.
>
> Why? It look OK to me. Of course one would expect This.Data to be
> initialised in Initialize so that This.Data is allways /= null...

This code says:
"If this pointer contains anything (not null) then make it point to
something new /without/ dealing with the original".

But it is still early... ;-)

Re: Memory leak - What the ...?

[Martin Krischik]

Martin Dowie wrote:

> Martin Krischik wrote:
>> Marius Amado Alves wrote:
>>
>>>>     if This.Data /= null then
>>>>          This.Data := new Stream_Element_Array'(This.Data.all);
>>>>       end if;
>>>
>>> To my pointerless eyes this looks very leaky.
>>
>> Why? It look OK to me. Of course one would expect This.Data to be
>> initialised in Initialize so that This.Data is allways /= null...
> 
> This code says:
> "If this pointer contains anything (not null) then make it point to
> something new /without/ dealing with the original".
 
> But it is still early... ;-)

Which should be Ok for Adjust. After all the Adjust standart tasks deep copy
- and not deep move. You can use Adjust for a deep move but that would be
special purpose.

A Finalise following the adjust should deal with the original.

With Regards

Martin

-- 
mailto://krischik@users.sourceforge.net
http://www.ada.krischik.com

Re: Memory leak - What the ...?

[Stephen Leake]

mosteo@gmail.com (Alex R. Mosteo) writes:

> Hi, 
> 
> as the topic says, this post is about some code which leaks. I'm now
> sure of having trapped the leak, but I don't understand where is my
> error.

Please post a complete compilable example, so I can run it with gnat 5.02a1.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Stephen Leake <stephen_leake@acm.org> wrote in message news:<mailman.270.1097458825.390.comp.lang.ada@ada-france.org>...
> mosteo@gmail.com (Alex R. Mosteo) writes:
> 
> > Hi, 
> > 
> > as the topic says, this post is about some code which leaks. I'm now
> > sure of having trapped the leak, but I don't understand where is my
> > error.
> 
> Please post a complete compilable example, so I can run it with gnat 5.02a1.

Here's the example. Gnatmem shows that it leaks heavily. So I must
have understood something really wrong about controlled types. I need
to get this right.

The example will create 1000 messages and then delete them
overwritting them with an empty value.

I suppose gnatchop will suffice (beware: use ./test and not simply
test):

with Ada.Finalization;
with Ada.Streams; use Ada.Streams;

package Test_Aux is
   
   type Stream_Element_Array_Access is access
      Ada.Streams.Stream_Element_Array;

   type Udp_Message is new Ada.Finalization.Controlled with record
      Data : Stream_Element_Array_Access;
   end record;

   ------------
   -- Create --
   ------------
   function Create (Data : in Stream_Element_Array) return
Udp_Message;

   procedure Adjust   (This : in out Udp_Message);
   procedure Finalize (This : in out Udp_Message);

end Test_Aux;


with Ada.Unchecked_Deallocation;

package body Test_Aux is

   ------------
   -- Create --
   ------------
   function Create (Data : in Stream_Element_Array) return Udp_Message
is
      Msg : Udp_Message := 
         (Ada.Finalization.Controlled with 
          Data => new Stream_Element_Array'(Data));
   begin
      return Msg;
   end Create;

   procedure Adjust   (This : in out Udp_Message) is
   begin
      if This.Data /= null then
         This.Data := new Stream_Element_Array'(This.Data.all);
      end if;
   end Adjust;

   procedure Finalize (This : in out Udp_Message) is
      procedure Free is new Ada.Unchecked_Deallocation (
         Stream_Element_Array, Stream_Element_Array_Access);
   begin
      Free (This.Data);
   end Finalize;

end Test_Aux;


with Ada.Exceptions;
with Ada.Streams; use Ada.Streams;
use  Ada;
with Text_IO; use Text_IO;

with Test_Aux; use Test_Aux;

procedure Test is
   Empty : Udp_Message;
   Arr   : array (1 .. 1000) of Udp_Message;
begin
   Text_Io.Put_Line ("Adding...");
   for I in Arr'Range loop
      Arr (I) := Create ((1 .. Stream_Element_Offset (I) =>
Stream_Element'First));
   end loop;

   Text_Io.Put_Line ("Deleting...");
   for I in Arr'Range loop
      Arr (I) := Empty;
   end loop;
exception
   when E: others =>
      Text_IO.Put_Line ("Exception: " &
Exceptions.Exception_Information (E));
end test;

Re: Memory leak - What the ...?

[Stephen Leake]

mosteo@gmail.com (Alex R. Mosteo) writes:

> Stephen Leake <stephen_leake@acm.org> wrote in message news:<mailman.270.1097458825.390.comp.lang.ada@ada-france.org>...
> > mosteo@gmail.com (Alex R. Mosteo) writes:
> > 
> > > Hi, 
> > > 
> > > as the topic says, this post is about some code which leaks. I'm now
> > > sure of having trapped the leak, but I don't understand where is my
> > > error.
> > 
> > Please post a complete compilable example, so I can run it with gnat 5.02a1.
> 
> Here's the example. Gnatmem shows that it leaks heavily. So I must
> have understood something really wrong about controlled types. I need
> to get this right.

Hmm. This inspired me to try GNAT.Debug_Pools. Here's the instrumented
code:

with Ada.Finalization;
with Ada.Streams;
with GNAT.Debug_Pools;
package Test_Aux is

   type Stream_Element_Array_Access is access Ada.Streams.Stream_Element_Array;
   Pool : GNAT.Debug_Pools.Debug_Pool;
   for Stream_Element_Array_Access'Storage_Pool use Pool;

   type Udp_Message is new Ada.Finalization.Controlled with record
      Data : Stream_Element_Array_Access;
   end record;

   function Create (Data : in Ada.Streams.Stream_Element_Array) return Udp_Message;

   procedure Adjust   (This : in out Udp_Message);
   procedure Finalize (This : in out Udp_Message);

end Test_Aux;
with Ada.Unchecked_Deallocation;
package body Test_Aux is

   function Create (Data : in Ada.Streams.Stream_Element_Array) return Udp_Message
   is
      Msg : Udp_Message :=
        (Ada.Finalization.Controlled with
           Data => new Ada.Streams.Stream_Element_Array'(Data));
   begin
      return Msg;
   end Create;

   procedure Adjust   (This : in out Udp_Message) is
   begin
      if This.Data /= null then
         This.Data := new Ada.Streams.Stream_Element_Array'(This.Data.all);
      end if;
   end Adjust;

   procedure Finalize (This : in out Udp_Message) is
      procedure Free is new Ada.Unchecked_Deallocation
        (Ada.Streams.Stream_Element_Array, Stream_Element_Array_Access);
   begin
      Free (This.Data);
   end Finalize;

end Test_Aux;
with Ada.Exceptions;
with Ada.Streams; use Ada.Streams;
with Ada.Text_IO; use Ada.Text_IO;
with Test_Aux; use Test_Aux;
with GNAT.Debug_Pools;
procedure Test is
   procedure Pool_Info is new GNAT.Debug_Pools.Print_Info (Put_Line);

   Empty : Udp_Message;
   Arr   : array (1 .. 1000) of Udp_Message;
begin
   Put_Line ("Adding...");
   for I in Arr'Range loop
      Arr (I) := Create ((1 .. Stream_Element_Offset (I) => Stream_Element'First));
   end loop;

   Put_Line ("Deleting...");
   for I in Arr'Range loop
      Arr (I) := Empty;
   end loop;

   Pool_Info (Pool);
exception
when E: others =>
   Put_Line ("Exception: " & Ada.Exceptions.Exception_Information (E));
end test;

Compiling with GNAT 5.02a1 and running, we get:

gnatmake -k -g -O0 -gnatf -gnato -gnatwa -gnatwe -gnatwL -gnatVa -I..  test -largs   -bargs -E -cargs 
gcc -c -I./ -g -O0 -gnatf -gnato -gnatwa -gnatwe -gnatwL -gnatVa -I.. -I- ..\test.adb
gcc -c -I./ -g -O0 -gnatf -gnato -gnatwa -gnatwe -gnatwL -gnatVa -I.. -I- ..\test_aux.adb
gnatbind -aO./ -I.. -E -I- -x test.ali
gnatlink test.ali -g
./test.exe 
Adding...
Deleting...
Total allocated bytes :  1530000
Total logically deallocated bytes :  1530000
Total physically deallocated bytes :  0
Current Water Mark:  0
High Water Mark:  511008

Which seems to indicate no leak.

I don't see anything obviously wrong with the code.

There may be a bug with this in GNAT 3.15p on your OS.

Try adding Text_IO.Put_Line in each call, and trace one object's
lifetime; that often makes things clear.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Brian May]

>>>>> "Stephen" == Stephen Leake <stephen_leake@acm.org> writes:

    Stephen> There may be a bug with this in GNAT 3.15p on your OS.

With Gnat 3.15p under Linux, I got:

Debug Pool info:
  Total allocated bytes :  1530000
  Total deallocated bytes :  1530000
  Current Water Mark:  0
  High Water Mark:  511008

Out of curiosity, I replaced the range of Arr to (1..1), and changed
the Create/Adjust/Finalize to print an appropriate Message; Now I get:

Adding...
(Create)(Adjust)(Finalize)(Finalize)(Adjust)(Finalize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)(Finalize)

Why does 1 create result in 2 adjusts and 3 finalize?
(I would have expected number of adjust == number of finalize == 1)

Why are there 2 finalize at the end?
(I would have expected 1 maximum)

Is there anyway of making this code more efficient?
(It seems to me that a deep copy for a create operation just causes
heap fragmentation with no real benefit.)
-- 
Brian May <bam@snoopy.apana.org.au>

Re: Memory leak - What the ...?

[Jean-Pierre Rosen]

Brian May a écrit :

> Why does 1 create result in 2 adjusts and 3 finalize?
> (I would have expected number of adjust == number of finalize == 1)
> 
> Why are there 2 finalize at the end?
> (I would have expected 1 maximum)
> 
> Is there anyway of making this code more efficient?
> (It seems to me that a deep copy for a create operation just causes
> heap fragmentation with no real benefit.)
The magic formula is:
Number_Of_Finalize = Number_Of_Initialize + Number_Of_Adjust + 
Number_Of_Aggregates

Remember that aggregates are objects which have no initialize (they are 
supposed to be created correctly). They are finalized, though.

-- 
---------------------------------------------------------
            J-P. Rosen (rosen@adalog.fr)
Visit Adalog's web site at http://www.adalog.fr

Re: Memory leak - What the ...?

[Marius Amado Alves]

Jean-Pierre Rosen wrote:

> Marius Amado Alves a �crit :
> 
>>
>> Jean-Pierre Rosen wrote:
>>
>> I still lack an understandable explanation of why this does not leak:
>>
>>       if This.Data /= null then
>>          This.Data := new Stream_Element_Array'(This.Data.all);
>>       end if;
>>
> Because the original data was declared as a field of an aggregate. When 
> this aggregate gets finalized, the corresponding data is freed.

Not the reason in the conclusion by the original poster, based on the 
discussion, that "this code is only valid because we are talking about 
an Adjust procedure of a Controlled type."

Either explanation is hardly understandable still.

Thanks anyway. I guess now I have something more concrete to chew on. 
I'll start by trying to locate the "original data" inside the bowl :-)

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Marius Amado Alves <amado.alves@netcabo.pt> wrote in message news:<mailman.282.1097576360.390.comp.lang.ada@ada-france.org>...

> I still lack an understandable explanation of why this does not leak:
> 
>        if This.Data /= null then
>           This.Data := new Stream_Element_Array'(This.Data.all);
>        end if;

Adjust is called just after the members of the record have been
copied. So imagine you have

A := B;

When Adjust(A) gets called, you will have

A.Data = B.Data

Since Data is an access type, you have a shallow copy where both A and
B have members pointing to the same heap space.

In the Adjust, what A does is replicate the pointed data to get a new
pointer and a deep copy where A and B point to different heap copies.

Additionally, before A being overwritten, Finalize(A) is called just
in case it needs to free some memory.

Hope this helps.

Re: Memory leak - What the ...?

[Marius Amado Alves]

>>I still lack an understandable explanation of why this does not leak:
>>
>>       if This.Data /= null then
>>          This.Data := new Stream_Element_Array'(This.Data.all);
>>       end if;
> 
> Adjust is called just after the members of the record have been
> copied. So imagine you have
> 
> A := B;
> 
> When Adjust(A) gets called, you will have
> 
> A.Data = B.Data
> 
> Since Data is an access type, you have a shallow copy where both A and
> B have members pointing to the same heap space.
> 
> In the Adjust, what A does is replicate the pointed data to get a new
> pointer and a deep copy where A and B point to different heap copies.
> 
> Additionally, before A being overwritten, Finalize(A) is called just
> in case it needs to free some memory.
> 
> Hope this helps.

Not really. The only way for leaking not to take place is for the old 
This.Data value being held somewhere. Where? Or for Finalize (A) to be 
called before Adjust, but then This.Data should become null, no? And 
then the if condition would never hold. The alternative being This.Data 
pointing to garbage.

Re: Memory leak - What the ...?

[Björn Persson]

Marius Amado Alves wrote:

>> Adjust is called just after the members of the record have been
>> copied. So imagine you have
>>
>> A := B;
>>
>> When Adjust(A) gets called, you will have
>>
>> A.Data = B.Data
>>
>> Since Data is an access type, you have a shallow copy where both A and
>> B have members pointing to the same heap space.
>>
>> In the Adjust, what A does is replicate the pointed data to get a new
>> pointer and a deep copy where A and B point to different heap copies.
>>
>> Additionally, before A being overwritten, Finalize(A) is called just
>> in case it needs to free some memory.
>>
>> Hope this helps.
> 
> Not really. The only way for leaking not to take place is for the old 
> This.Data value being held somewhere. Where?

In B, where it's been all the time. If B was a variable on the stack, 
it's still there on the stack, and still points to the old copy of the 
data. If B was itself on the heap, then obviously there's a pointer to B 
somewhere, since it could be accessed and copied to A. That pointer is 
still there, and still points to B, which in turn still points to the 
old copy of the data. If B was a temporary object, then the compiler is 
smart enough to finalize it after A has been adjusted (or else the 
compiler is broken).

-- 
Björn Persson                              PGP key A88682FD
                    omb jor ers @sv ge.
                    r o.b n.p son eri nu

Re: Memory leak - What the ...?

[Marius Amado Alves]

>> ... The only way for leaking not to take place is for the old 
>> This.Data value being held somewhere. Where?
> 
> In B, where it's been all the time. ...
> ... If B was a temporary object, then the compiler is 
> smart enough to finalize it after A has been adjusted (or else the 
> compiler is broken).

Ok. This is probably what confuses me, that Finalize is called after, 
since Adjust is supposed to be the *last* step of an assignment 7.6 (2).

Thanks.

Re: Memory leak - What the ...?

[Stephen Leake]

Marius Amado Alves <amado.alves@netcabo.pt> writes:

> >> ... The only way for leaking not to take place is for the old
> >> This.Data value being held somewhere. Where?
> > In B, where it's been all the time. ...
> > ... If B was a temporary object, then the compiler is smart enough
> > to finalize it after A has been adjusted (or else the compiler is
> > broken).
> 
> Ok. This is probably what confuses me, that Finalize is called after,
> since Adjust is supposed to be the *last* step of an assignment 7.6
> (2).

Finalize is not called as the last step of assignement. Finalize (B)
is called when B goes out of scope.

For example:

declare
    A : foo;
    C : foo;
begin
    declare
        B : foo := initial;
    begin
        A := B; -- A Adjusted
        C := B; -- C Adjusted
        ... other code
    end; -- B finalized
end; -- A, C finalized

-- 
-- Stephe

Re: Memory leak - What the ...?

[Marius Amado Alves]

> Finalize is not called as the last step of assignement. Finalize (B)
> is called when B goes out of scope.

Of course you're right. Thanks for laying down the whole schematics :-)

Re: Memory leak - What the ...?

[Stephen Leake]

Marius Amado Alves <amado.alves@netcabo.pt> writes:

> >>I still lack an understandable explanation of why this does not leak:
> >>
> >>       if This.Data /= null then
> >>          This.Data := new Stream_Element_Array'(This.Data.all);
> >>       end if;
> > Adjust is called just after the members of the record have been
> > copied. So imagine you have
> > A := B;
> > When Adjust(A) gets called, you will have
> > A.Data = B.Data
> > Since Data is an access type, you have a shallow copy where both A
> > and
> > B have members pointing to the same heap space.
> > In the Adjust, what A does is replicate the pointed data to get a new
> > pointer and a deep copy where A and B point to different heap copies.
> > Additionally, before A being overwritten, Finalize(A) is called just
> > in case it needs to free some memory.
> > Hope this helps.
> 
> Not really. The only way for leaking not to take place is for the old
> This.Data value being held somewhere. Where? 

In B. Eventually, Finalize (B) is called.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Stephen Leake]

Marius Amado Alves <amado.alves@netcabo.pt> writes:

> Jean-Pierre Rosen wrote:
> 
> > Brian May a écrit :
> >
> >> Why does 1 create result in 2 adjusts and 3 finalize?
> >> (I would have expected number of adjust == number of finalize == 1)
> >>
> >> Why are there 2 finalize at the end?
> >> (I would have expected 1 maximum)
> >>
> >> Is there anyway of making this code more efficient?
> >> (It seems to me that a deep copy for a create operation just causes
> >> heap fragmentation with no real benefit.)
> > The magic formula is:
> > Number_Of_Finalize = Number_Of_Initialize + Number_Of_Adjust +
> > Number_Of_Aggregates
> > Remember that aggregates are objects which have no initialize (they
> > are supposed to be created correctly). They are finalized, though.
> 
> One of the areas where Ada is absolutely counter-intuitive and
> troublesome :-(

What would you consider "intuitive" in the area of Ada.Controlled?
"intuitive" means "something known without learning". How can you
expect anyone to know how Ada.Controlled works without learning it?

I suspect you actually mean "not like anything else I know". Well,
yes!

> Controledness in Ada is a bowl of spagetti.

No, I think it's more like a mildly complex electronics schematic.
Confusing at first, but wonderfully powerful and functional once you
understand it.

> I still lack an understandable explanation of why this does not
> leak:
> 
>        if This.Data /= null then
>           This.Data := new Stream_Element_Array'(This.Data.all);
>        end if;

When this code is called (in Adjust), This.Data is a _copy_ of a
pointer (the copy is made by the initial object copy done before
Adjust is called). The assignment throws away the copy of the pointer,
and replaces it with a pointer to a copy of the data.

The original copy of the original pointer is still in the original
object, and will be finalized.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Hyman Rosen]

Stephen Leake wrote:
> What would you consider "intuitive" in the area of Ada.Controlled?
> I suspect you actually mean "not like anything else I know".

Most importantly, not like C++. Ada's model of breaking controlled
assignment into piecemeal function calls with a bit copy in the
middle is difficult to understand and deal with, as is the possibility
that Finalize may be called on an object multiple times, as is the
fact that Initialize isn't called on aggregates. In C++, for example,
user-defined assignment has access to both the object being assigned
and the object and value being assigned from. In Ada, if the identity
of the source object is important, I suppose you would have to finagle
something with access discriminants (but I don't know Ada, so I'm not
really sure).

Re: Memory leak - What the ...?

[Martin Krischik]

Hyman Rosen wrote:

> Stephen Leake wrote:
>> What would you consider "intuitive" in the area of Ada.Controlled?
>> I suspect you actually mean "not like anything else I know".
> 
> Most importantly, not like C++. Ada's model of breaking controlled
> assignment into piecemeal function calls with a bit copy in the
> middle is difficult to understand and deal with, as is the possibility
> that Finalize may be called on an object multiple times, as is the
> fact that Initialize isn't called on aggregates. In C++, for example,
> user-defined assignment has access to both the object being assigned
> and the object and value being assigned from. In Ada, if the identity
> of the source object is important, I suppose you would have to finagle
> something with access discriminants (but I don't know Ada, so I'm not
> really sure).

There is some advantage over C++ as well: It should render better
performance on current CPU's and is saver. The reason is the all or nothing
approach of C++. Either you use the build in bit copy constructor and
assignment operator or you define you.

1st: block copys are faster on most CPUs then single assignments.

2nd: It is an easy mistake to forget the copy constructor or assignment
operator when adding a new attribute. The copy constructor will then
default initialise and assignment operator will keep the old value. And
that is wrong behaviour in 99.9% of all cases.

And please don't tell me you might *sometimes* want that behaviour. That is
the precise point I just hate about C++: For the 0.1% change that you need
some special behaviour the language accepts faulty code in cases where the
compiler could actually find the bug for you.

With Regards

Martin
-- 
mailto://krischik@users.sourceforge.net
http://www.ada.krischik.com

Re: Memory leak - What the ...?

[Hyman Rosen]

Martin Krischik wrote:
> Either you use the build in bit copy constructor and
> assignment operator or you define you.

The compiler-generated copy constructor and assignment
operator are defined to do memberwise copying, not bit
copies. The compiler may implement these more efficiently
as it chooses, but it can do that for user-defined ones
as well.

> 2nd: It is an easy mistake to forget the copy constructor or assignment
> operator when adding a new attribute. And please don't tell me you might
 > *sometimes* want that behaviour.

You should avoid writing your own assignment and copy
constructors when you possibly can, and use the default
ones. If some member requires special copy handling,
then it should be of a type which encapsulates this.
Then adding new members doesn't require making any
changes to the copying code.

Re: Memory leak - What the ...?

[Dmitry A. Kazakov]

On Wed, 13 Oct 2004 07:52:59 +0100, Marius Amado Alves wrote:

> At top level the abstractions of initialization, finalization and 
> assignment are totally intuitive.

They are not. Ada.Finalization is a hack to be replaced by a better, safer,
more universal model.

-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

Re: Memory leak - What the ...?

[Mark Lorenzen]

"Dmitry A. Kazakov" <mailbox@dmitry-kazakov.de> writes:

> On Wed, 13 Oct 2004 07:52:59 +0100, Marius Amado Alves wrote:
> 
> > At top level the abstractions of initialization, finalization and 
> > assignment are totally intuitive.
> 
> They are not. Ada.Finalization is a hack to be replaced by a better, safer,
> more universal model.
> 
> -- 
> Regards,
> Dmitry A. Kazakov
> http://www.dmitry-kazakov.de

Is there an AI on this?

Regards,
- Mark Lorenzen

Re: Memory leak - What the ...?

[Dmitry A. Kazakov]

On 13 Oct 2004 19:44:35 +0200, Mark Lorenzen wrote:

> "Dmitry A. Kazakov" <mailbox@dmitry-kazakov.de> writes:
> 
>> On Wed, 13 Oct 2004 07:52:59 +0100, Marius Amado Alves wrote:
>> 
>>> At top level the abstractions of initialization, finalization and 
>>> assignment are totally intuitive.
>> 
>> They are not. Ada.Finalization is a hack to be replaced by a better, safer,
>> more universal model.
> 
> Is there an AI on this?

I think, it would be difficult to approach this problem by small to
moderate steps, as the AI procedure is supposed to work. In any case one
should first reach some consensus on the requirements. In my view they are:

1. All types should provide an ability to control initialization,
finalization and copy (if not limited). This should include *all* types
without any exceptions. Noticeable, the class-wide types as well (9,
below).

2. Assignment and construction should be considered separate. It should be
possible to have true in-place constructors. The programmer should have a
choice between assignments automatically generated from copy constructors
and completely overridden ":=".

3. Initialization Object : X := 1; is not assignment!

4. There should be constructors with parameters.

5. Assignment should have access to the left side.

6. Assignment should be a primitive operation and so dispatch on both
sides.

7. There should be no need to call explicitly parent's default constructor
("Initialize"), destructor ("Finalize"), copy constructor ("Adjust"). Very
desirable to have some "extensible subroutine" mechanics invented and
deployed in the language.

8. There should be also no way to call a constructor or destructor
explicitly.

9. The problem of dispatch during initialization/finalization should be
considered paramount. In my view the only unambiguous and sane way to solve
it is when class-wides would have constructors/destructors of their own. In
this case dispatch will happen in a class-wide constructor, when all
necessary specific objects are already constructed. Similarly, class-wide
finalization will be made before ultimate finalization of specific objects.

10. Aggregates should be user-definable. Initialization by an aggregate
should be considered as a constructor.

11. User abstract factory (a-la S'Class'Input) should be available.

Have I forgot something? And isn't it too much for an AI? (:-))

-- 
Regards,
Dmitry A. Kazakov
http://www.dmitry-kazakov.de

Re: Memory leak - What the ...?

[Brian May]

>>>>> "Jean-Pierre" == Jean-Pierre Rosen <rosen@adalog.fr> writes:

    Jean-Pierre> The magic formula is:
    Jean-Pierre> Number_Of_Finalize = Number_Of_Initialize + Number_Of_Adjust +
    Jean-Pierre> Number_Of_Aggregates

    Jean-Pierre> Remember that aggregates are objects which have no
    Jean-Pierre> initialize (they are supposed to be created
    Jean-Pierre> correctly). They are finalized, though.

So in this code:

(Create)(Adjust)(Finalize)(Finalize)(Adjust)(Finalize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)(Finalize)


If my counting is correct:

1 Create.
3 Adjust.
6 Finalize.


Or

1 + 3 + Number_Of_Aggregates = 6

Or

Number_Of_Aggregates = 2

I am still confused as to why 2 objects would be created without
either a Create or an Adjust.

Ok, there is the initial value, before it is assigned. That makes
1. Not 2.

I guess my confusion comes from the two finalize at the end.
-- 
Brian May <bam@snoopy.apana.org.au>

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Brian May <bam@snoopy.apana.org.au> wrote in message news:<sa4655ghc5h.fsf@snoopy.apana.org.au>...
> >>>>> "Stephen" == Stephen Leake <stephen_leake@acm.org> writes:
> 
>     Stephen> There may be a bug with this in GNAT 3.15p on your OS.
> 
> With Gnat 3.15p under Linux, I got:
> 
> Debug Pool info:
>   Total allocated bytes :  1530000
>   Total deallocated bytes :  1530000
>   Current Water Mark:  0
>   High Water Mark:  511008

First off, let me thanks everyone of you for your help. Getting in
someone's code is sometimes a bore. I regret that I only have access
via google to the news by the moment, so the discussion can't be more
fluid :(

I think some points have been raised that are already clear. The Data
pointer is not initialized because for uninitialized ones, pointers
value is null in Ada by default. Second, this code is only valid
because we are talking about an Adjust procedure of a Controlled type.
Third, we agree in that there's no logic flaw (what a relief!).

Now some results. I've tested your gnat.debug_pools code (only in
win32, which is the only one I have available until thursday) and I
get:

Debug Pool info:
  Total allocated bytes :  1530000
  Total deallocated bytes :  1530000
  Current Water Mark:  0
  High Water Mark:  511008

That's it, no leak reported. But running

gnatmem 3 test.exe

I get:

Global information
------------------
   Total number of allocations        :6001
   Total number of deallocations      :   0
   Final Water Mark (non freed mem)   :   1.49 Megabytes
   High Water Mark                    :   1.49 Megabytes

<snip of stacktraces>

Which clearly shows... not a single deallocation !? (BTW the result is
similar without the Debug_Pool). This is wrong. That's the reason I
firstly stated the test case was leaking. Too fast jump to
conclusions. But linking with -lgmem and running the post-mortem
gnatmem

gnatmem 10 -i gmem.out test.exe

I get:

Global information
------------------
   Total number of allocations        :3001
   Total number of deallocations      :3000
   Final Water Mark (non freed mem)   :  24 Bytes
   High Water Mark                    : 499.05 Kilobytes

Allocation Root # 1
-------------------
 Number of non freed allocations    :   1
 Final Water Mark (non freed mem)   :  24 Bytes
 High Water Mark                    :  24 Bytes
 Backtrace                          :
   a-except.adb:1289 ada.exceptions.process_raise_exception
   a-except.adb:2638 <ada__exceptions___elabb>
   b~test.adb:106    adainit
   b~test.adb:189    main

Which is consistent.

I've placed traces in adjust and finalize and they're called as
expected (some of you have already reproduced this).

However, one thing is absolutely true: changing this type by an all
static one has removed the leak in my full program. Since we can agree
that the implementation is right, I must be doing something different
in my program which this test case doesn't show. I'll try to summarize
all accesses to this type for you, or better still to get a faultly
test case.

I started this bug chase because I have a program which apparently
runs fine but that after 24 hours of uptime starts to bog down a
server (not under my control, one in which I can't get memory usage)
so it seems plausible that the leak is making it to swap like crazy. I
didn't suspect of a leak until I observed it in windows, and I haven't
been back to linux since.

Finally, here are posted the memory-usage functions I use in both
Linux and Win32, which you can find of interest.

-------- Win32 version (requires win32ada binding) --------

with Interfaces;
with Interfaces.C;
with System;

with Win32; 
use  Win32;
with Win32.Winbase;
with Win32.Winerror;
with Win32.Winnt;

package body Adagio.Os.Memory is

   ------------------------------------------------------------------------
   -- LocalHeapSize                                                   
  --
   ------------------------------------------------------------------------
   function LocalHeapSize (Heap : in Win32.Winnt.HANDLE) return
Natural is
      use Interfaces;
      use Interfaces.C;
      use Win32.Winbase;
      use Win32.Winerror;
      use Win32.Winnt;

      he     : aliased PROCESS_HEAP_ENTRY;
      Size   : Natural := 0;
      Blocks : Natural := 0;
   begin
      he.lpData := System.Null_address;
      loop
         if HeapWalk (Heap, he'unchecked_access) = 0 then
            if GetLastError = ERROR_NO_MORE_ITEMS then
               exit;
            else
               raise Walk_error;
            end if;
         end if;
         Blocks := Blocks + 1;
         if (Unsigned_32 (he.wFlags) and 
            Unsigned_32 (PROCESS_HEAP_UNCOMMITTED_RANGE)) = 0 
         then
            Size := Size + Natural (he.cbData);
         end if;
      end loop;

      return Size;
   end LocalHeapSize;

   ------------------------------------------------------------------------
   -- MemorySize                                                      
  --
   ------------------------------------------------------------------------
   function MemorySize return Natural is
      use Win32.Winbase;
      use Win32.Winnt;

      heaps  : array (1 .. 100) of aliased HANDLE;
      nHeaps : DWORD;
      cSize  : Natural := 0;
   begin
      nHeaps := 
         GetProcessHeaps (heaps'length, heaps
(heaps'first)'Unchecked_access);

      if Natural (nHeaps) = heaps'Last then
         raise Constraint_error;
      end if;

      for N in 1 .. Natural (nHeaps) loop
         cSize := cSize + LocalHeapSize (heaps (N));
      end loop;
         
      return cSize;
   end MemorySize;

   ------------------------------------------------------------------------
   -- Heap_usage                                                      
  --
   ------------------------------------------------------------------------
   -- Returns the heap memory in use in bytes
   function Heap_usage return Natural is
   begin
      return MemorySize;
   end Heap_usage;

end Adagio.Os.Memory;

----- Linux version, requires access to /proc filesystem
---------------
-- There are some auxiliary functions missing, but they're very basic:
-- A integer-to-string without leading space conversion, 
-- a extract-column-from-line... 

   function Get_Pid return Integer;
   pragma Import (C, Get_Pid, "__getpid");

   ------------------------------------------------------------------------
   -- Heap_usage                                                      
  --
   ------------------------------------------------------------------------
   -- Returns the heap memory in use in bytes
   -- Any thread pid is OK, since all threads share the memory info
(?)
   function Heap_usage return Natural is
      Pid : constant Integer := Get_Pid;
      use Text_IO;
      File : File_Type;
      Line : String (1 .. 256);
      Last : Natural;
   begin
      Open (File, Name => "/proc/" & Agpl.Strings.To_String (Pid) & 
                                     "/statm", Mode => In_file);
      Get_Line (File, Line, Last);
      Close (File);

      -- The second number in /proc/<pid>/statm gives the resident
memory size
      return 
         1024 * 
         Natural'Value (
            Agpl.Strings.Fields.Select_Field (Line (1 .. Last), 2));
   exception
      when others =>
         if Is_Open (File) then
            Close (File);
         end if;
         return 0;
   end Heap_usage;

Re: Memory leak - What the ...?

[Stephen Leake]

mosteo@gmail.com (Alex R. Mosteo) writes:

> That's it, no leak reported. But running
> 
> gnatmem 3 test.exe
> 
> I get:
> 
> Global information
> ------------------
>    Total number of allocations        :6001
>    Total number of deallocations      :   0
>    Final Water Mark (non freed mem)   :   1.49 Megabytes
>    High Water Mark                    :   1.49 Megabytes
> 
> <snip of stacktraces>
> 
> Which clearly shows... not a single deallocation !? 

SO there's a bug (or feature) in gnatmem on Windows. I suspect GNAT
makes one large request from the OS, and doesn't return it until after
gnatmem finishes. But I'm just guessing.

> (BTW the result is similar without the Debug_Pool). This is wrong.

"wrong" by what GNAT reference manual? "not what I expected", perhaps.
"Wrong" requires a clear definition of what is "right", from the
compiler documentation.

> That's the reason I firstly stated the test case was leaking. Too
> fast jump to conclusions. But linking with -lgmem and running the
> post-mortem gnatmem
> 
> gnatmem 10 -i gmem.out test.exe
> 
> I get:
> 
> Global information
> ------------------
>    Total number of allocations        :3001
>    Total number of deallocations      :3000
>    Final Water Mark (non freed mem)   :  24 Bytes
>    High Water Mark                    : 499.05 Kilobytes
> 
> Allocation Root # 1
> -------------------
>  Number of non freed allocations    :   1
>  Final Water Mark (non freed mem)   :  24 Bytes
>  High Water Mark                    :  24 Bytes
>  Backtrace                          :
>    a-except.adb:1289 ada.exceptions.process_raise_exception
>    a-except.adb:2638 <ada__exceptions___elabb>
>    b~test.adb:106    adainit
>    b~test.adb:189    main
> 
> Which is consistent.

Ok.

> I've placed traces in adjust and finalize and they're called as
> expected (some of you have already reproduced this).
> 
> However, one thing is absolutely true: changing this type by an all
> static one has removed the leak in my full program. 

What leak?

Use Gnat.Debug_Pools for that, as well.

> Since we can agree that the implementation is right, I must be doing
> something different in my program which this test case doesn't show.
> I'll try to summarize all accesses to this type for you, or better
> still to get a faultly test case.
> 
> I started this bug chase because I have a program which apparently
> runs fine but that after 24 hours of uptime starts to bog down a
> server (not under my control, one in which I can't get memory usage)
> so it seems plausible that the leak is making it to swap like crazy. I
> didn't suspect of a leak until I observed it in windows, and I haven't
> been back to linux since.

This could also be heap fragmentation, rather than a leak.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Pascal Obry]

Stephen Leake <stephen_leake@acm.org> writes:

> SO there's a bug (or feature) in gnatmem on Windows. 

NO gnatmem works quite fine on Windows. I use all the time as I have a
check_mem non regression test in AWS. So it is probably something else.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

Re: Memory leak - What the ...? - FOUND

[Alex R. Mosteo]

Stephen Leake <stephen_leake@acm.org> wrote in message news:<mailman.293.1097626382.390.comp.lang.ada@ada-france.org>...
> mosteo@gmail.com (Alex R. Mosteo) writes:
> 
> > That's it, no leak reported. But running
> > 
> > gnatmem 3 test.exe
> > 
> > I get:
> > 
> > Global information
> > ------------------
> >    Total number of allocations        :6001
> >    Total number of deallocations      :   0
> >    Final Water Mark (non freed mem)   :   1.49 Megabytes
> >    High Water Mark                    :   1.49 Megabytes
> > 
> > <snip of stacktraces>
> > 
> > Which clearly shows... not a single deallocation !? 
> 
> SO there's a bug (or feature) in gnatmem on Windows. I suspect GNAT
> makes one large request from the OS, and doesn't return it until after
> gnatmem finishes. But I'm just guessing.
> 
> > (BTW the result is similar without the Debug_Pool). This is wrong.
> 
> "wrong" by what GNAT reference manual? "not what I expected", perhaps.
> "Wrong" requires a clear definition of what is "right", from the
> compiler documentation.

Wrong in the sense that that's not what's happening, as Debug_Pools
and the post-mortem gnatmem show. I guess this is some GDB integration
problem, since it is run transparently in this live mode.

According to the user guide, the two gnatmem modes should reflect the
same behavior:

"Both modes produce the very same output." [Gnat UG]

> > However, one thing is absolutely true: changing this type by an all
> > static one has removed the leak in my full program. 
> 
> What leak?
> 
> Use Gnat.Debug_Pools for that, as well.

I know that my position is feeble, since I haven't been able to prove
the leak. But, my fault (see below) or gnat, there's a leak in my
original code :/

After testing with Gnat.Debug_Pools, here are the findings:

* gnatmem says the culprit is the Udp_Message.Data member.
* Debug_Pool says everything is okay with these allocations (no leak).
* ?????

> This could also be heap fragmentation, rather than a leak.

Would then gnatmem show unreleased allocations? I don't think so.

In the course of writing this post (several hours going in and out)
I've found the problem. You will want to beat me :( I had already a
Debug_Pool in place for the Stream_Element_Array_Access, which is
declared in another package, and which I hadn't look at.

So there's no leak but the normal operation of the Debug_Pools, which
consume memory monotonically up (I already knew this but alas I had
not thought of it). Interesting that gnatmem doesn't show any trace of
Gnat.Debug_Pools code. Of course, when I was not using this array
access, the pool wasn't being used and so the apparent leak
dissapeared.

I'm very very sorry for having wasted your time. At least some
interesting points on Controlled types have been discussed. I
certainly have benefited from it.

Kind regards,

A. Mosteo.

Re: Memory leak - What the ...?

[Brian May]

>>>>> "Alex" == Alex R Mosteo <mosteo@gmail.com> writes:

    Alex> Total number of allocations        :3001
    Alex> Total number of deallocations      :3000
    Alex> Final Water Mark (non freed mem)   :  24 Bytes
    Alex> High Water Mark                    : 499.05 Kilobytes

If it:
a) allocates 3000 bytes.
b) allocates 1 byte.
c) frees the 3000 bytes allocation.

You may find, you effectively still have 3001 bytes allocated (as far
as the OS is concerned) because of memory fragmentation. The key
issue, if you allocate 3000 bytes, does it reuse the memory or
allocate more memory?

Anyway, just a random thought; it may not be related to your issues in
anyway. Nor have I research how modern memory allocation algorithms
work anyway.
-- 
Brian May <bam@snoopy.apana.org.au>

Re: Memory leak - What the ...?

[Matthew Heaney]

Brian May <bam@snoopy.apana.org.au> writes:

> Why does 1 create result in 2 adjusts and 3 finalize?
> (I would have expected number of adjust == number of finalize == 1)

Create is probably creating a controlled temporary, that is immediately
destroyed following the assignment.


> Why are there 2 finalize at the end?
> (I would have expected 1 maximum)
> 
> Is there anyway of making this code more efficient?
> (It seems to me that a deep copy for a create operation just causes
> heap fragmentation with no real benefit.)

Implement Create as I showed in my previous posts, then rerun your
program and post your results.

Re: Memory leak - What the ...?

[Brian May]

>>>>> "Matthew" == Matthew Heaney <matthewjheaney@earthlink.net> writes:

    Matthew> Implement Create as I showed in my previous posts, then rerun your
    Matthew> program and post your results.

I already did, and got the same results.

(hmm... I thought I already reported this; maybe I forgot; oh well).
-- 
Brian May <bam@snoopy.apana.org.au>

Re: Memory leak - What the ...?

[Matthew Heaney]

Stephen Leake <stephen_leake@acm.org> writes:

>    function Create (Data : in Ada.Streams.Stream_Element_Array) 
>        return Udp_Message
>    is
>       Msg : Udp_Message :=
>         (Ada.Finalization.Controlled with
>            Data => new Ada.Streams.Stream_Element_Array'(Data));
>    begin
>       return Msg;
>    end Create;

Implement Create this way:

  function Create (Data : in Ada.Streams.Stream_Element_Array) 
     return Udp_Message is
  begin
     return (Controlled with new Stream_Element_Array'(Data));
  end Create;

Rerun your program with the change above and post your results.

Re: Memory leak - What the ...?

[Martin Krischik]

Matthew Heaney wrote:

> Stephen Leake <stephen_leake@acm.org> writes:
> 
>>    function Create (Data : in Ada.Streams.Stream_Element_Array)
>>        return Udp_Message
>>    is
>>       Msg : Udp_Message :=
>>         (Ada.Finalization.Controlled with
>>            Data => new Ada.Streams.Stream_Element_Array'(Data));
>>    begin
>>       return Msg;
>>    end Create;
> 
> Implement Create this way:
> 
>   function Create (Data : in Ada.Streams.Stream_Element_Array)
>      return Udp_Message is
>   begin
>      return (Controlled with new Stream_Element_Array'(Data));
>   end Create;
> 
> Rerun your program with the change above and post your results.

Well I don't really see a difference - apart from being move verbose and
easier to debug. Once the optimiser discovers that Msg is only used once
the same code should be the same.

With Regards

Martin

-- 
mailto://krischik@users.sourceforge.net
http://www.ada.krischik.com

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Matthew Heaney <matthewjheaney@earthlink.net> wrote in message news:<u655f4a95.fsf@earthlink.net>...
> Stephen Leake <stephen_leake@acm.org> writes:
> 
> >    function Create (Data : in Ada.Streams.Stream_Element_Array) 
> >        return Udp_Message
> >    is
> >       Msg : Udp_Message :=
> >         (Ada.Finalization.Controlled with
> >            Data => new Ada.Streams.Stream_Element_Array'(Data));
> >    begin
> >       return Msg;
> >    end Create;
> 
> Implement Create this way:
> 
>   function Create (Data : in Ada.Streams.Stream_Element_Array) 
>      return Udp_Message is
>   begin
>      return (Controlled with new Stream_Element_Array'(Data));
>   end Create;
> 
> Rerun your program with the change above and post your results.

You're so right. However it doesn't seem to make a difference for the
leak. I'm going to try with the Debug_Pool now.

Thanks!

Re: Memory leak - What the ...?

[Matthew Heaney]

mosteo@gmail.com (Alex R. Mosteo) writes:

>    function Create (Data : in Stream_Element_Array) return Udp_Message
> is
>       Msg : Udp_Message := 
>          (Ada.Finalization.Controlled with 
>           Data => new Stream_Element_Array'(Data));
>    begin
>       return Msg;
>    end Create;

You should never implement a constructor for a controlled type this way.
Do it this way instead:

function Create (Data : in Stream_Element_Array) return Udp_Message is
begin
   return (Controlled with new String_Element_Array'(Data));
end Create;

Re: Memory leak - What the ...?

[Stephen Leake]

Matthew Heaney <matthewjheaney@earthlink.net> writes:

> mosteo@gmail.com (Alex R. Mosteo) writes:
> 
> >    function Create (Data : in Stream_Element_Array) return Udp_Message
> > is
> >       Msg : Udp_Message := 
> >          (Ada.Finalization.Controlled with 
> >           Data => new Stream_Element_Array'(Data));
> >    begin
> >       return Msg;
> >    end Create;
> 
> You should never implement a constructor for a controlled type this way.
> Do it this way instead:
> 
> function Create (Data : in Stream_Element_Array) return Udp_Message is
> begin
>    return (Controlled with new String_Element_Array'(Data));
> end Create;

Surely any halfway decent compiler would optimize these to be the
same?

Hmm. Perhaps declaring an object requires calling Initialize, Adjust,
and Finalize, unless the compiler can _prove_ those subprograms have
no side-effects, which it probably can't do. Is that your point?

It helps to explain rationale, rather than just laying down rules.

-- 
-- Stephe

Re: Memory leak - What the ...?

[Matthew Heaney]

"Stephen Leake" <stephen_leake@acm.org> wrote in message
news:mailman.304.1097670377.390.comp.lang.ada@ada-france.org...
> Surely any halfway decent compiler would optimize these to be the
> same?
>
> Hmm. Perhaps declaring an object requires calling Initialize, Adjust,
> and Finalize, unless the compiler can _prove_ those subprograms have
> no side-effects, which it probably can't do. Is that your point?

Something like that.  If I have this:

function Op return CT is --some controlled type
   O : CT;
begin
   O.X := ...;
   return O;
end;

then local controlled object O must be Initialize'd when it is declared, and
Finalize'd when the scope of its declaration ends.

If you do it this way:

function Op return CT is
begin
   return CT'(Controlled with X => ...);
end;

then the compiler can probably build the return value in place:

declare
    O : CT := Op;
begin

This is similar to the return-value optimization in C++.

As another example, the AI-302 set functions Union, Intersection, etc, are
implemented this way:

function Union (L, R : Set) return Set is
   Tree : Tree_Type;  --ordered set uses red-black tree (a type which isn't
controlled)
begin
   ... -- build Tree
   return Set'(Controlled with Tree);
end;

Re: Memory leak - What the ...?

[Brian May]

>>>>> "Matthew" == Matthew Heaney <mheaney@on2.com> writes:

    Matthew> function Op return CT is begin return CT'(Controlled with
    Matthew> X => ...); end;

    Matthew> then the compiler can probably build the return value in
    Matthew> place:


On my system, I get identical results:

Adding...
(Create)(Adjust)(Finalize)(Finalize)(Adjust)(Finalize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)(Finalize)

curious.

--------------------------
What I would have hoped:
--------------------------
Adding...
(Create)(Finialize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)

Justification:


Arr(1) := Create(...); expands into:

+ Temp := Create(...); -- shallow copy
+ Finalize(Arr(1));
+ Arr(1) := Temp;      -- shallow copy


Arr(1) := Empty; expands into:

+ Temp := Empty;       -- shallow copy
+ Finalize(Arr(1));
+ Arr(1) := Temp;      -- shallow copy
+ Adjust(Arr(1))

on exit:

+ Finalize(Arr(1));


As I would have assumed it should be obvious that a deep copy is not
required when the original value is going to be finalised.

--------------------
What I might expect:
--------------------
Adding...
(Create)(Finalize)(Adjust)(Finalize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)


Justification:

Arr(1) := Create(...) expands into:

+ Temp := Create(...);  -- shallow copy
+ Finalize(Arr(1));
+ Arr(1) := Temp;       -- shallow copy
+ Adjust(Arr(1));
+ Finalize(Temp);

Arr(1) := Empty expands into:

+ Temp := Empty;        -- shallow copy
+ Finalize(Arr(1));
+ Arr(1) := Temp;       -- shallow copy
+ Adjust(Arr(1));

on exit:

+ Finalize(Arr(1));

i.e. to answer someone else's question Adjust is required to copy the
value, and the original value is no longer required, so it is
finalised.


-----------
what I got 
-----------
(Create)(Adjust)(Finalize)(Finalize)(Adjust)(Finalize)Deleting...
(Finalize)(Adjust)Debug Pool info:
  Total allocated bytes :  36
  Total deallocated bytes :  36
  Current Water Mark:  0
  High Water Mark:  24

(Finalize)(Finalize)



which leaves "(Adjust)(Finalize)" after create unaccounted for (maybe
this is the return statement?).

It also leaves the last (Finalize) unaccounted for.

This seems very inefficient.
-- 
Brian May <bam@snoopy.apana.org.au>

Re: Memory leak - What the ...?

[Jeffrey Carter]

Stephen Leake wrote:

> Hmm. Perhaps declaring an object requires calling Initialize, Adjust,
> and Finalize, unless the compiler can _prove_ those subprograms have
> no side-effects, which it probably can't do. Is that your point?

Certainly true of Limited_Controlled. Consider a "safe" semaphore. The 
object is never referenced, but Initialize obtains the semaphore, and 
Finalize releases is.

-- 
Jeff Carter
"Mr. President, we must not allow a mine-shaft gap!"
Dr. Strangelove
33

Re: Memory leak - What the ...?

[Martin Dowie]

Alex R. Mosteo wrote:
>       if This.Data /= null then

You have a 'wood from trees' problem with this line... :-)

-- Martin

Re: Memory leak - What the ...?

[Alex R. Mosteo]

"Martin Dowie" <martin.dowie@baesystems.com> wrote in message news:<416a3d06_1@baen1673807.greenlnk.net>...
> Alex R. Mosteo wrote:
> >       if This.Data /= null then
> 
> You have a 'wood from trees' problem with this line... :-)

I don't get it :)

Re: Memory leak - What the ...?

[Alex R. Mosteo]

Well, I have collected all the code snippets which use Udp_Message.
This is part of an UDP bandwidth throttling mechanism, here stripped
to only leave the relevant parts. I can't reproduce it with a small
test case. I can't see anything wrong in this code neither. I'll try
with Valgrind when I get back to my linux PC, but I feel unsuccesfull.

I've placed counters in the Udp_Message type that show that there
aren't leaked objects. The leak affects only the Data buffers, not the
whole objects.

Thanks everyone for your time. Code follows.

package Udp_Msgs_List is new Ada.Containers.Doubly_Linked_Lists (
   Udp_Message);

Udp_Msgs : Udp_Msgs_List.List;

-- Packet creation: --------------------
   Udp_message_list.Prepend (
      Udp_msgs,
      Create (Buffer (1 .. Last), Packet.Destination));
----------------------------------------

-- Sending -----------------------------
-- This is called periodically following some throttling rules.
---------------------------------------------------------------
   if Is_empty (Udp_msgs) then
      Next_sending := Clock + 1.0;
   else
      Msg := First_Element (Udp_msgs);
      Delete_first (Udp_msgs);

      Socket.Send (
         Udp_Socket,
         Msg.Data (Msg.Data'First .. Msg.Last),
         Last, -- Out value with the last sent component (should be
Msg.Last)
         Msg.Dest);

      -- Drop old packets:
      loop
         I := Udp_message_list.Last (Udp_msgs);
         exit when Is_empty (Udp_msgs) or else
            Now - Element (I).Date <=
            Globals.Options.G2_UdpOutboundTimeout;
         Delete_last (Udp_msgs); -- Drop it!
      end loop;
   end if;

Re: Memory leak - What the ...?

[Matthew Heaney]

"Alex R. Mosteo" <mosteo@gmail.com> wrote in message
news:dcb57d1e.0410120707.256204ff@posting.google.com...

>    if Is_empty (Udp_msgs) then
>       Next_sending := Clock + 1.0;
>    else
>       Msg := First_Element (Udp_msgs);
>       Delete_first (Udp_msgs);
>
>       Socket.Send (
>          Udp_Socket,
>          Msg.Data (Msg.Data'First .. Msg.Last),
>          Last, -- Out value with the last sent component (should be
> Msg.Last)
>          Msg.Dest);

Note that function First_Element is going to make a copy of the element.
Your element type is controlled, and uses allocation, so this isn't going to
be cheap.  I recommend instead that you refer to the element in place:

else

   declare
        procedure Send (Msg : UDP_Msg_T) is
        begin
          Send (UDP_Socket, Msg.Data (...), ...);
       end;
    begin
        Query_Element (First (UDP_Msgs), Send'Access); --Ada 2005
    end;

    Delete_First (UDP_Msgs);
   ...
end if;