Memory leak When Returning TJSONVALUE from Function

Question

I have a JSON array of objects that I iterate. From each element of the array I retrieve further json data via a function that returns a JSONValue. I then must add that returned jsonvalue to the specific array element. It works perfectly except that Delphi is reporting a memory leak on shutdown and I just can't find it. I'm pretty new to JSON and I've spent all day on this. Can someone help by suggesting the right way to go about this. Simplified code below. Many Thanks.

function TForm1.Function1(ThisProductJSON: String): Integer;
Var
  MultiObj    : TJSONObject;
  OtherValues : TJSONVALUE;
  ThisObject  : TJSONObject;
  I           : Integer;
  jsArr       : TJSONARRAY;

  S:String;
  Stockcode : String;
begin
  MultiObj:=TJSONObject.Create(nil);
  s:='[{"ThisElement":1,"Thiscode":12345,"ThisBarcode":"2345678901231","Price":1.00,"Measure":"EA","Description":"Some Description"},'+
      '{"ThisElement":2,"Thiscode":21345,"ThisBarcode":"3124567890123","Price":2.00,"Measure":"EA","Description":"Some Description"},'+
      '{"ThisElement":3,"Thiscode":31345,"ThisBarcode":"6123457890123","Price":3.00,"Measure":"EA","Description":"Some Description"},'+
      '{"ThisElement":4,"Thiscode":41345,"ThisBarcode":"9123456780123","Price":4.00,"Measure":"EA","Description":"Some Description"},'+
      '{"ThisElement":5,"Thiscode":51345,"ThisBarcode":"8234567901235","Price":5.00,"Measure":"EA","Description":"Some Description"}]';
  ThisProductJSON:=S;
  try
    try

      MultiObj.AddPair('ThisName','ThisValue');
      MultiObj.AddPair('AnotherName','AnotherValue');
      I:=0;
      JSArr:=TJSONObject.ParseJSONValue(ThisProductJSON).AsType<TJSONARRAY>;
            //Process all products in the array with a call to detailed Rest for each product
             begin
                for I := 0 to jsArr.count-1 do //iterate through the array
                  begin

                    Stockcode:= jsArr.Items[I].GetValue<string>('Thiscode');

                    ThisObject:=TJSONObject(jsArr[i]); // Make ThisObject Point to the jsArr[i] as an object so I can add the Other Values to this array element

                    OtherValues :=(GetOtherDetails(Stockcode)); // Call the below function to return the JSONVALUE containing addtional data

                    ThisObject.AddPair('OtherDetails',OtherValues); // Add all of the additional data in OtherValues JSONVALUE to this element of array via the object
                  end;
                MultiObj.AddPair('WoWMultiProduct',JSARR); // This MultiObj hokds the consolidated data
              end;


    except
      On E: Exception do
        begin
           //Errror handling here
        end;
    end;

  finally
    MultiObj.Free;
    OtherValues.Free; // <<- I thought this would free the Result of the function but I think its leaking
    JSARR.Free;
    if assigned(ThisObject) then ThisObject.Free;
  end;
end;

function TForm1.GetOtherDetails(Stockcode: String): TJSONVALUE;
Var
  ThisResponseObject, MYRESULT  : TJSONOBJECT;
  vNIP, vCOO          : TJSONVALUE;

begin

  DetailedRESTRequest.execute;  //<-- This REST Service Returns aditional JSON data
  MyResult:=TJSONObject.Create;
  if DetailedRESTResponse.StatusCode=200 then
     begin
        Try

          Try
             ThisResponseObject := TJSONObject.ParseJSONValue(DetailedRESTResponse.content)as TJSONObject; // Resd the entire JSON Response into TJSONObject
              vCOO:=ThisResponseObject.Getvalue('COO'); // Get First JSON Value I need
              vNIP:=ThisResponseObject.Getvalue('Nip');  // Get Next JSON Value I Need
              MyResult.AddPair('NIPInfo',vNip); // Thiis is the only way I know how to Add the Values
              MyResult.AddPair('COO',vCOO);     // So that I can get them both together in the Result

              Result:= TJSONObject.ParseJSONValue(MyResult.ToJSON)as TJSONValue; // Get back the JSONValue from the function

          Except
              On E:Exception do
                begin
                  // Some Error Handling Here e.Classname+' ' +E.Message ;

                end;
          End;
        Finally
          If Assigned(ThisResponseObject) then ThisResponseObject.Free;
          // NOTE IF I FREE MYRESULT OBJECT I GET ACCESS VIOLATION WHEN I TRY TO USE THE RESULT OF THE FUNCTION
        End;

     end;

end;

Leaving the memory leak aside, your finally is broken because in case of exceptions you may be referring to uninitialized variables. Further, even without exceptions ThisObject may be uninitialized. I think you need to step back and learn the correct patterns for lifetime management. — David Heffernan, Jul 29 '21 at 11:13
@David Heffernan Thanks David, I've looked for as many examples as I can find and I usually live by the rule of thumb of "if I create it, I free it". I would usually wrap all of the objects in the finally in an If assigned condition. If there's more that I should be doing could you show me using the above code as a baseline? The memory leak is my foremost issue though, but I would like to use good practices. Many Thanks. — JohnT, Jul 29 '21 at 12:25
You need multiple finally blocks, one for each object, unless you initialise variables to nil. But websearch will find plenty of good articles. — David Heffernan, Jul 29 '21 at 16:05

score 0 · Answer 1 · answered May 13 '22 at 16:30

JSON objects are a tree-like data structure. When you request a value from a node, with getvalue (for example), it is actually giving you a reference to that node's object.

so, when you do the following in the "GetOtherDetails" function:

vCOO:=ThisResponseObject.Getvalue('COO');
MyResult.AddPair('COO',vCOO);

You make ThisResponseObject and MyResult share nodes (memory locations), so when you free one of them the other will try to free memory locations that no longer exist and generate the access violation

ThisResponseObject.free;
MyResult.Free; //access violation

Similarly, on "Function1" when doing:

ThisObject:=TJSONObject(jsArr[i]);
OtherValues :=(GetOtherDetails(Stockcode)); 
ThisObject.AddPair('OtherDetails',OtherValues);

You're making ThisObject contain OtherValues object... so when you try to free the two objects you're going to run into memory problems.

Memory leak When Returning TJSONVALUE from Function

1 Answers1