How to correctly handle the finalization of limited records?

Hello

I hope you don’t mind programming questions as I haven’t seen many of them on this forum. I am implementing a vector using container aggregates and a limited type. The finalization of the elements is managed externally.

I don’t understand why Finalize is called twice here? How do I have to handle the finalization of the vector in the right way?

Thank you.

with Ada.Finalization;
with Ada.Text_IO;
with Ada.Unchecked_Deallocation;
procedure Test is

   generic
      type Element_Type (<>) is private;
      type Index_Type is range <>;
   package Vector is

      type Vector_Type is tagged limited private
         with Aggregate => (Empty => Empty1, Add_Unnamed => Append);

      function Empty1 (Size : Index_Type) return Vector_Type;
      function Empty2 return Vector_Type; -- gives an "error: left hand of assignment must not be limited type"

      procedure Append
        (V : in out Vector_Type;
         New_Element : access Element_Type);

      function Get (V : in out Vector_Type; Index : Index_Type) return Element_Type;

   private

      type Elements_Array is array (Index_Type range <>) of access Element_Type;
      type Elements is access Elements_Array;

      type Vector_Type is
         new Ada.Finalization.Limited_Controlled with record
            Data  : Elements;
            Index : Index_Type;
         end record;

      overriding
      procedure Finalize (V : in out Vector_Type);

      function Get (V : in out Vector_Type; Index : Index_Type) return Element_Type is
        (V.Data (Index).all);

   end Vector;

   package body Vector is

      function Empty1 (Size : Index_Type) return Vector_Type is
        (Ada.Finalization.Limited_Controlled with Data => new Elements_Array (1 .. Size), Index => 1);

      function Empty2 return Vector_Type is
        (Ada.Finalization.Limited_Controlled with Data => new Elements_Array (1 .. 3), Index => 1);

      procedure Append
        (V : in out Vector_Type;
         New_Element : access Element_Type) is
      begin
         V.Data (V.Index) := New_Element;
         V.Index := @ + 1;
      end Append;

      overriding
      procedure Finalize (V : in out Vector_Type) is
         procedure Free is new Ada.Unchecked_Deallocation (Elements_Array, Elements);
      begin
         Ada.Text_IO.Put_Line ("finalize called..." & V'Image & V.Data'Image);

         if V.Data /= null then -- is this if necessary?
            Free (V.Data);
            Ada.Text_IO.Put_Line ("finalized!");
         end if;
      end Finalize;

   end Vector;

begin

   declare
      type Element is tagged record
         A_Number : Integer;
      end record;

      package Elements is new Vector (Element, Positive);
      Element1 : aliased Element := (A_Number => 1);
      Element2 : aliased Element := (A_Number => 2);
      Element3 : aliased Element := (A_Number => 3);
      E : Elements.Vector_Type := [Element1'Access, Element2'Access, Element3'Access];
   begin
      Ada.Text_IO.Put_Line (E'Image);
      Ada.Text_IO.Put_Line (E.Get (1)'Image);
      Ada.Text_IO.Put_Line (E.Get (2)'Image);
      Ada.Text_IO.Put_Line (E.Get (3)'Image);
   end;

end Test;

Console output

{TEST.B_1.ELEMENTS.VECTOR_TYPE object}

(A_NUMBER =>  1)

(A_NUMBER =>  2)

(A_NUMBER =>  3)
finalize called...{TEST.B_1.ELEMENTS.VECTOR_TYPE object}(access 600003e95208)
finalized!
finalize called...{TEST.B_1.ELEMENTS.VECTOR_TYPE object}(access 600003e95208)
test(50414,0x1fcbbf240) malloc: *** error for object 0x600003e95200: pointer being freed was not allocated
test(50414,0x1fcbbf240) malloc: *** set a breakpoint in malloc_error_break to debug

raised PROGRAM_ERROR : unhandled signal

I don’t know the actual “why” but the RM requires that finalize be idempotent so that it can be called multiple times. My guess is for some compilers there is an easier finalization implementation that can make use of that fact. Also since you can call Finalize manually, it is always a good idea to make sure Finalize is callable multiple times safely.

See paragraph 24 and 24a: Completion and Finalization

3 Likes

Making Finalize idempotent is generally a good thing (so reset any pointer to null for instance). But here it is already done by Unchecked_Deallocation.

The issue is in fact that you have two vectors sharing the same pointer apparently (which is strange for a limited type, I think there might be an issue in GNAT’s implementation of the Aggregate aspect).

Changing your code to

      E : Elements.Vector_Type := Elements.Empty2;
   begin
      E.Append (Element1'Access);
      E.Append (Element2'Access);
      E.Append (Element3'Access);

fixes the issue. Finalize is now called once once.

In fact, I believe that for limited controlled types, Finalize is only ever called once, that’s one of the major reasons for making controlled types limited when you can (because otherwise as @jere mentioned the compiler is free to call Finalize multiple times).

If I may add a few more comments on your code:

  • I think Append should not receive an access type in parameter, but make a copy of the element. Using too many access types is often a bad sign in Ada (this is close to C style). It will also be easier for you to manage memory. In your example, what if I do E.Append (new Element). you have no way to know this memory should be freed, but not in E.Append (Element1'Access).
  • Append should initialize the vector if not done yet (in my rewrite, I had to assign Empty1 otherwise I am getting an error. This is more user-friendly and in line with what the standard Ada containers are doing.
2 Likes

Thank you for your responses.

I thought, that my Finalize was already idempotent… There is even a null check and yet the if-block is called twice. Or have I made a mistake in my code? What does an idempotent procedure look like?

@ebriot Somehow I also had the feeling that it could be a compiler bug. For a while I thought I hadn’t understood the concept of limited types, as there shouldn’t be any copies. Is it worth reporting this error?

My next step is to implement a storage pool with some kind of garbage collection. I know that access types are not appreciated, but I want to keep the number of copies low for now.

It’s idempotent. I was just bringing it up because it leans into the RM allowing multiple calls, so even if you don’t want multiple calls you do want to ensure it can handle situations where they are required or allowed.

I wasn’t sure if it was a bug or intentional in your case (ebriot covered that), but I figured I would at least provide that information for future planning since you were getting into this area and already worried about multiple calls.

1 Like

It is not necessary. Freeing a null access value has no effect. See ARM 13.11.2(9/5).

1 Like

You are using a user-defined Vector_Type aggregate […], that creates a new Vector_Type object. Then you assign it to E. That calls Finalize on the right part. Your implementation of Finalize frees all elements, so all access types in the right part become dangling pointers. When E gets finalized you get well-deserved Program_Error.
Morale: avoid access types. If you must, use smart pointers. Otherwise, treat very carefully…

1 Like

Dmitry,
What you describe is likely what’s happening indeed. But his vector type is limited, so construction should happen in place, and there should be no “right part”. That’s where I think there is a bug in GNAT (perhaps I missed something, of course)

1 Like

Welcome @gast and @dmitry-kazakov to the forums!

I am sorry that I have nothing else to add to this thread…

Best,
Fer

1 Like

I agree that this looks like a bug.

1 Like

I have reported this bug to AdaCore, as I can see us using this aggregate at some point, and anything dealing with incorrect implementation of controlled types is something we are very sensitive to.

4 Likes

AdaCore has now fixed that bug in their implementation, I guess it will be merged at some point in gcc.

3 Likes

Has this patch been merged into gcc yet? gcc.gnu.org Git - gcc.git/history - gcc/ada

Just wondering, shouldn’t the Initialize & Finalize cancel-out for a single-line declaration and initialisation, even in the non-limited case?

Ah! Just saw the bug report. :slight_smile:

Bug has been fixed… I tested it with gcc 15.0.0 20241115 :+1:

3 Likes