1669 System instability

Beta Testing for Windows Products and plugins

Moderator: Gurus

markstuartwalker
Posts: 931
Joined: Fri Jul 10, 2009 8:10 am

Re: 1669 System instability

Post by markstuartwalker »

I think that I have pinned down that the root of the memory corruption. I have some cache space which is gutting overrun and it follows this memory usage.

The following works but I can't figure out if it is adequately allocating memory that it returns back to MM. Please give me some comments.

Code: Select all

function DeviceiTunesListPlaylists(DeviceHandle :Integer): PScanResult;
type
  PFileName = ^TFileName;
  TFileName = array [0 .. 255] of WideChar;
var
  lst: TStringList;
  i, size: integer;
  name: String;
  name2: PFileName;
begin
  // list of playlists please
  Lst := getListOfPlaylists(dev);

  // allocate the memory for the reply back
  new(Result);
  Result.FileCount := lst.count;
  size := lst.count * sizeof(PScanRecord);
  GetMem(Result^.Files,size);

  log.text(2, 'number and size', lst.count , size);

  // iterate through the list ...
  for i := 0 to lst.count - 1 do
  begin
    name := lst[i];
    log.text(2, i, 'playlist', name);
    new(name2);
    StrCopy(@name2[1], PWideChar(name));
    new(Result.Files[i]);
    Result.Files[i].filename := @name2[1];
    Result.Files[i].filesize := 0;
    Result.Files[i].lastmodified := 0;
    Result.Files[i].track_cookie := 0;

  end;
Windows 7,8 / Ubuntu 13.10 / Mavericks 10.9 / iOS 7.1 / iTunes 11.1
iTunes plugin (d_itunes & itunes4) http://www.mediamonkey.com/forum/viewto ... =2&t=45713
Running MM under Mac OS X with Wine http://www.mediamonkey.com/forum/viewto ... =4&t=58507
Ludek
Posts: 4959
Joined: Fri Mar 09, 2007 9:00 am

Re: 1669 System instability

Post by Ludek »

Mark yes, this is most probably the root of the problem.

You are not allocating memory for name2.

For example our iPod plugin uses this code:

Code: Select all

function TiPodDB.GetScannedPlaylists: PScanResult;
var
  i : integer;
  s : String;
  plst : TiPodPlaylist;
begin
  EnterCriticalSection( iPodCS);
  try
    ClearScannedPlaylists;
    New( ScannedPlaylists);
    GetMem( ScannedPlaylists.Files, playlists.Count*sizeof(ScannedPlaylists.Files[0]));
    ScannedPlaylists.FileCount := 0;
    for i:=0 to playlists.Count-1 do
    begin
      plst := TiPodPlaylist( playlists[i]);
      if plst.MHYP.podcast_flag = 0 then
      begin
        New( ScannedPlaylists.Files[ScannedPlaylists.FileCount]);
        ZeroMemory( ScannedPlaylists.Files[ScannedPlaylists.FileCount], sizeof(ScannedPlaylists.Files[ScannedPlaylists.FileCount]^));
        s := plst.Title;
        GetMem( ScannedPlaylists.Files[ScannedPlaylists.FileCount].filename, length(s)*2+2);
        StrCopy( ScannedPlaylists.Files[ScannedPlaylists.FileCount].filename, PWideChar( s));
        inc( ScannedPlaylists.FileCount);
      end;
    end;
    result := ScannedPlaylists;
  finally
    LeaveCriticalSection( iPodCS);
  end;
end;

procedure TiPodDB.ClearScannedPlaylists;
var
  i : integer;
begin
  EnterCriticalSection( iPodCS);
  try
    if ScannedPlaylists<>nil then
    begin
      for i:=0 to ScannedPlaylists^.FileCount-1 do
      begin
        Dispose( ScannedPlaylists^.Files^[i].filename);
        Dispose( ScannedPlaylists^.Files^[i]);
      end;
      Dispose( ScannedPlaylists^.Files);
      Dispose( ScannedPlaylists);
      ScannedPlaylists := nil;
    end;
  finally
    LeaveCriticalSection( iPodCS);
  end;
end;
where ScannedPlaylists : PScanResult is private member of class TiPodDB, and corresponds to Result struct in your code.
As you can see each filename gets its memory via GetMem() and is subsequently freed via Dispose() in TiPodDB.ClearScannedPlaylists.

So doing something similar should solve your troubles. Hope that helps.
markstuartwalker
Posts: 931
Joined: Fri Jul 10, 2009 8:10 am

Re: 1669 System instability

Post by markstuartwalker »

OK, I think that I can see that now. I have created a few functions to encapsulate this capability. Please review and confirm.

Q1: Do I need Critical sections in my code, I understood the plugins to be single threaded?
Q2: ZeroMemory() is not in standard libraries but I think that is of negligible benefit
Q3: Doesn't MM dispose of the memory once it has consumed the information? I do not retain a pointer the the results that I return from either of these functions.

Code: Select all

unit ScanResultUnit;

interface

uses sysutils,DeviceCommon;

function createScanResult(count:integer):PScanResult;
procedure addScanResult(res:PScanResult;
i:Integer;
                        name:string ;
                        filesize:integer ;
                        lastModified:TDateTime ;
                        track_cookie:integer);

implementation


function createScanResult(count:integer):PScanResult;
var
  size:integer;
begin
  new(Result);
  Result.FileCount := count;
  size := count * sizeof(Result.Files[0]);
  GetMem(Result.Files,size);

(*
  New( ScannedPlaylists);
    GetMem( ScannedPlaylists.Files, playlists.Count*sizeof(ScannedPlaylists.Files[0]));
*)

end;

procedure addScanResult(res:PScanResult;
                        i:Integer;
                        name:string ;
                        filesize:integer ;
                        lastModified:TDateTime ;
                        track_cookie:integer);
type
  PFileName = ^TFileName;
  TFileName = array [0 .. 255] of WideChar;
var
//  name2 : PfileName ;
  rec : PScanRecord;
begin
  if i < res.FileCount then
  begin

    new(res.Files[i]);
    rec:=res.files[i]; // for debug
//    ZeroMemory( res.Files[i], sizeof(res.Files[i]^));

    //new(name2);
    //StrCopy(@name2[1], PWideChar(name));
    GetMem( res.Files[i].filename, length(name)*2+2);
    StrCopy( res.Files[i].filename, PWideChar(name));

//    res.Files[i].filename := @name2[1];
    res.Files[i].filesize := filesize;
    res.Files[i].lastmodified := lastModified;
    res.Files[i].track_cookie := track_cookie;
  end;
end;

(*
function TiPodDB.GetScannedPlaylists: PScanResult;
var
  i : integer;
  s : String;
  plst : TiPodPlaylist;
begin
  EnterCriticalSection( iPodCS);
  try
    ClearScannedPlaylists;
    New( ScannedPlaylists);
    GetMem( ScannedPlaylists.Files, playlists.Count*sizeof(ScannedPlaylists.Files[0]));
    ScannedPlaylists.FileCount := 0;
    for i:=0 to playlists.Count-1 do
    begin
      plst := TiPodPlaylist( playlists[i]);
      if plst.MHYP.podcast_flag = 0 then
      begin
        New( ScannedPlaylists.Files[ScannedPlaylists.FileCount]);
        ZeroMemory( ScannedPlaylists.Files[ScannedPlaylists.FileCount], sizeof(ScannedPlaylists.Files[ScannedPlaylists.FileCount]^));
        s := plst.Title;
        GetMem( ScannedPlaylists.Files[ScannedPlaylists.FileCount].filename, length(s)*2+2);
        StrCopy( ScannedPlaylists.Files[ScannedPlaylists.FileCount].filename, PWideChar( s));
        inc( ScannedPlaylists.FileCount);
      end;
    end;
    result := ScannedPlaylists;
  finally
    LeaveCriticalSection( iPodCS);
  end;
end;

procedure TiPodDB.ClearScannedPlaylists;
var
  i : integer;
begin
  EnterCriticalSection( iPodCS);
  try
    if ScannedPlaylists<>nil then
    begin
      for i:=0 to ScannedPlaylists^.FileCount-1 do
      begin
        Dispose( ScannedPlaylists^.Files^[i].filename);
        Dispose( ScannedPlaylists^.Files^[i]);
      end;
      Dispose( ScannedPlaylists^.Files);
      Dispose( ScannedPlaylists);
      ScannedPlaylists := nil;
    end;
  finally
    LeaveCriticalSection( iPodCS);
  end;
end;*)

end.
Windows 7,8 / Ubuntu 13.10 / Mavericks 10.9 / iOS 7.1 / iTunes 11.1
iTunes plugin (d_itunes & itunes4) http://www.mediamonkey.com/forum/viewto ... =2&t=45713
Running MM under Mac OS X with Wine http://www.mediamonkey.com/forum/viewto ... =4&t=58507
Ludek
Posts: 4959
Joined: Fri Mar 09, 2007 9:00 am

Re: 1669 System instability

Post by Ludek »

markstuartwalker wrote: Q1: Do I need Critical sections in my code, I understood the plugins to be single threaded?
Yes, if your plugin is single threaded then you don't need a CS (I just copied the code from one of our native plugins)
markstuartwalker wrote: Q2: ZeroMemory() is not in standard libraries but I think that is of negligible benefit
ZeroMemory is part of Windows.pas unit that is part of Dephi 2010. It only fills the memory by zeros.

Code: Select all

procedure ZeroMemory(Destination: Pointer; Length: DWORD);
begin
  FillChar(Destination^, Length, 0);
end;
markstuartwalker wrote: Q3: Doesn't MM dispose of the memory once it has consumed the information? I do not retain a pointer the the results that I return from either of these functions.
No, the plugin is responsible to dispose the memory that it allocated. So if you doesn't dispose then there is a memory leak.
markstuartwalker
Posts: 931
Joined: Fri Jul 10, 2009 8:10 am

Re: 1669 System instability

Post by markstuartwalker »

I have added the necessary memory releases etc. These are within 4.1.0.2960 which I posted today. The mis-behaving of the internal cache has gone.

I still get the exception upon exit in the Dev environment.
Windows 7,8 / Ubuntu 13.10 / Mavericks 10.9 / iOS 7.1 / iTunes 11.1
iTunes plugin (d_itunes & itunes4) http://www.mediamonkey.com/forum/viewto ... =2&t=45713
Running MM under Mac OS X with Wine http://www.mediamonkey.com/forum/viewto ... =4&t=58507
Post Reply