jsonrestdelphidelphi-11-alexandria

Delphi REST and JSON classes - to free or not to free?


I have the following procedure that refreshes access and refresh tokens from Xero accounting.

If I uncomment the jObject.Free line, then the xeroResponse.Free line causes an Access Violation.

I have looked at other code doing similar things, and some free the TJSONObject and TRESTClient, TRESTRequest and TRESTResponse objects. And some don't.

The code below works, but I'm not sure if I have memory leaks because I'm not freeing something I should be. And, is there an order I need to free things in? Or, does the below procedure have flaws and should be written differently?

And, do TJSONValue and TJSONArray need to be freed or not?

procedure CheckXeroTokenValidity;
var
  xeroClient: TRESTClient;
  xeroRequest: TRESTRequest;
  xeroResponse: TRESTResponse;
  jObject : TJSONObject;
  sRefreshToken : string;
begin
  //this procedure will check to see if the access token is older than 30 minutes and will refresh it, if the refresh token if not more than 60 days old
  with dmProcs do begin
    ResetTable(tXero, '', True);
    tXero.First;
    if not tXero.Eof then begin
      //check to see if the access token is expired or almost expired - if it is, refresh it if the refresh token is less than 60 days old
      if MinutesBetween(tXeroDateToken.AsDateTime, Now) > 25 then begin
        //check to see if the refresh token is still valid
        if DaysBetween(tXeroDateToken.AsDateTime, Date) < 59 then begin
          //get the current refresh token
          sRefreshToken := tXeroRefreshToken.AsString;

          xeroClient := TRESTClient.Create(nil);
          xeroRequest := TRESTRequest.Create(nil);
          xeroResponse := TRESTResponse.Create(nil);

          Screen.Cursor := crHourGlass;
          try
            xeroClient.ContentType := 'application/json';
            xeroResponse.ContentType := 'application/json';
            xeroRequest.Client := xeroClient;
            xeroRequest.Response := xeroResponse;
            xeroRequest.Method := TRESTRequestMethod.rmPOST;
            xeroRequest.Params.Clear;
            xeroRequest.Params.AddItem('grant_type', 'refresh_token');
            xeroRequest.Params.AddItem('client_id', sClientID);
            xeroRequest.Params.AddItem('refresh_token', sRefreshToken);
            xeroClient.BaseURL := 'https://identity.xero.com/connect/token';

            try
              xeroRequest.Execute;
              if Assigned(xeroResponse.JSONValue) then begin

                //parse the JSON
                jObject := TJSONObject.Create;
                jObject := xeroResponse.JSONValue as TJSONObject;
                if (xeroResponse.StatusCode = 200) and (Assigned(jObject.GetValue('access_token'))) then begin
                  try
                    tXero.Edit;
                    tXeroAccessToken.AsString := jObject.GetValue('access_token').Value;
                    tXeroRefreshToken.AsString := jObject.GetValue('refresh_token').Value;
                    tXeroDateToken.AsDateTime := Now;
                    tXero.Post;
                  except on E:Exception do
                    ShowMessage('Error: ' + E.Message);
                  end;
                end else begin
                  //if an error occured or a new access and refresh token was not supplied, then delete the xero record
                  tXero.First;
                  if not tXero.Eof then begin
                    tXero.Delete;
                  end;

                  if Assigned(jObject.GetValue('error')) then begin
                    ShowMessage('Error: ' + jObject.GetValue('error').ToString);
                  end;
                end;
              end;
            except on E:Exception do
              ShowMessage('Error: ' + xeroResponse.ErrorMessage + ' - ' + E.Message);
            end;
          finally
            Screen.Cursor := crDefault;
            //jObject.Free;
            xeroClient.Free;
            xeroRequest.Free;
            xeroResponse.Free;
          end;
        end else begin
          Information('Please see your manager to re-establish your accounting connection.');
        end;
      end;
    end;
  end;
end;

Solution

  • If I uncomment the jObject.Free line, then the xeroResponse.Free line causes an Access Violation.

    You are freeing a TJOSNObject that TRESTResponse owns, and tries to free after you have already freed it.

    The code below works, but I'm not sure if I have memory leaks because I'm not freeing something I should be.

    Yes, you do have a memory leak. You are creating your own TJSONObject, and then discarding (leaking) it to use the TJSONObject that TRESTResponse provides to you. Get rid of the TJSONObject.Create statement entirely, you don't need it.

    And, do TJSONValue and TJSONArray need to be freed or not?

    In this situation, no.

    TRESTResponse.JSONValue is owned by TRESTResponse, so you do not need to worry about freeing it. And that TJSONObject owns its sub-values, so you don't need to worry about freeing them, either.

    The only objects that need to be freed in this code are the REST objects you are creating yourself. And even then, I would suggest you make them own each other, so you only have 1 object to free yourself, eg:

    xeroClient := TRESTClient.Create(nil);
    xeroRequest := TRESTRequest.Create(xeroClient);
    xeroResponse := TRESTResponse.Create(xeroClient);
    ...
    xeroClient.Free;
    

    FYI, on a side note:

    //if an error occured or a new access and refresh token was not supplied, then delete the xero record
    tXero.First;
    if not tXero.Eof then begin
      tXero.Delete;
    end;
    

    You are already on the 1st record at this point in the code, so there is no need to call tXero.First() or check tXero.Eof here. You are processing only the 1st record in the DataSet, you are not looping through the DataSet, but even if you were, going back to the 1st record to delete it would be wrong. You should be deleting only the active record that you are processing.