anymore1113 / superobject

Automatically exported from code.google.com/p/superobject
0 stars 0 forks source link

Memory leak in TSuperRttiContext.AsType<T> #53

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Hello!

You have the following code in your function:
...
var
  ret: TValue;
begin
  if FromJson(TypeInfo(T), obj, ret) then
    Result := ret.AsType<T>
  else
    raise Exception.Create('Marshalling error');
end;

if FromJson fails because of any reason (say characters instead of digits for 
integer field etc..) then instance of the type is leaked. as you just raise the 
exception without releasing the instance in the FomrJson or before raise...

Program to reproduce the fail:

program JsonFail;

{$APPTYPE CONSOLE}

uses
  FastMM4,
  superobject,
  System.SysUtils;

type
  JSON = class sealed
    class function Store<T>(AObject: T; AIdent: Boolean = True): String; static;
    class function Load<T>(AJSON: String): T; static;
  end;

  TTest = class
  public
    x: Integer;
  end;

{ JSON }

class function JSON.Load<T>(AJSON: String): T;
begin
  with TSuperRttiContext.Create do
    try
      Result := AsType<T>(SO(AJSON));
    finally
      Free;
    end;
end;

class function JSON.Store<T>(AObject: T; AIdent: Boolean): String;
begin
  with TSuperRttiContext.Create do
    try
      Result := AsJson<T>(AObject).AsJson(AIdent);
    finally
      Free;
    end;
end;

var
  T, N: TTest;
  s: String;
begin
  FastMM4.SuppressMessageBoxes := False;
  try
    T := TTest.Create;
    try
      T.x := 1;
      s := JSON.Store(T);
      Writeln(s);
    finally
      T.Free;
    end;
    s := StringReplace(s, '1', 'x', []);
    N := JSON.Load<TTest>(s);
    try
    finally
      N.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
  Readln;
end.program JsonFail;

{$APPTYPE CONSOLE}

uses
  FastMM4,
  superobject,
  System.SysUtils;

type
  JSON = class sealed
    class function Store<T>(AObject: T; AIdent: Boolean = True): String; static;
    class function Load<T>(AJSON: String): T; static;
  end;

  TTest = class
  public
    x: Integer;
  end;

{ JSON }

class function JSON.Load<T>(AJSON: String): T;
begin
  with TSuperRttiContext.Create do
    try
      Result := AsType<T>(SO(AJSON));
    finally
      Free;
    end;
end;

class function JSON.Store<T>(AObject: T; AIdent: Boolean): String;
begin
  with TSuperRttiContext.Create do
    try
      Result := AsJson<T>(AObject).AsJson(AIdent);
    finally
      Free;
    end;
end;

var
  T, N: TTest;
  s: String;
begin
  FastMM4.SuppressMessageBoxes := False;
  try
    T := TTest.Create;
    try
      T.x := 1;
      s := JSON.Store(T);
      Writeln(s);
    finally
      T.Free;
    end;
    s := StringReplace(s, '1', 'x', []);
    N := JSON.Load<TTest>(s);
    try
    finally
      N.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
  Readln;
end.

So in the procdure FromClass you should set the try except block.. and release 
the instance if something fails.

Hope that helps you to improve your library.

Original issue reported on code.google.com by Zigmund....@gmail.com on 24 Feb 2014 at 4:38

GoogleCodeExporter commented 9 years ago
Actully FromClass does not help for me. I added release in the AsType method as:

function TSuperRttiContext.AsType<T>(const obj: ISuperObject): T;
var
  ret: TValue;
begin
  if FromJson(TypeInfo(T), obj, ret) then
    Result := ret.AsType<T>
  else
    begin
      if ret.Kind = tkClass then
        ret.AsObject.Free;
      raise Exception.Create('Marshalling error');
    end;
end;

(not sure if it's safe...)

Original comment by Zigmund....@gmail.com on 24 Feb 2014 at 4:44

GoogleCodeExporter commented 9 years ago
And more thing. You use "raise Exception.Create('')" - not a best idea. Can you 
type your exception with some base exception class so that other can filter out 
the excpetion by it's type and not just exception..

ESuperException = class(Exception) will be enough..

Much better when handling errors.

Original comment by Zigmund....@gmail.com on 24 Feb 2014 at 4:46