Zwolnienie pamięci w TFileStream za pomocą .FREE oraz FreeAndNil powodują losowe zawieszenie działania programu

0

Witam!
Od dwóch dni próbuje pojąć dlaczego mój program w losowych momentach przestaje działać.

Opis działania kodu.
Wątek SupervisorThread przygotowuje listę plików do połączenia w ChunksToCombineList (TStringList). Nowe wpisy są oczywiście dodawane w kotekście głównego wątku (Synchronize()).
Po dodaniu nowej pozycji w ChunksToCombineList wznawiany jest uśpiony wątek CombineChunksThread.

procedure TSupervisorThread.Execute;
begin
  
  while Terminated=false do
  begin  
    Synchronize(UpdateListOfChunksToCombine);
    sleep(250);
  end;
  
end;

procedure TSupervisorThread.UpdateListOfChunksToCombine;
begin

  while NextChunkToCombineIndex<form1.JvListView1.Items.Count do
  begin
     if Pos('encoded',form1.JvListView1.Items.Item[NextChunkToCombineIndex].SubItems.Strings[1])<>0 then
     begin
       ChunksToCombineList.Add(IntToStr(NextChunkToCombineIndex+1));
       if (Assigned(CombineChunksThread)=true) and (CombineChunksThread.Suspended=true) then CombineChunksThread.Resume;
       Inc(NextChunkToCombineIndex);
     end
     else break;
  end;
 
end;
procedure TCombineChunksThread.Execute;
begin

  repeat

    Synchronize(CheckChunksToCombine);
    if (ChunkToCombine<>'') and (CombineChunk(ChunkToCombine)=true) then Inc(NumberOfCombinedChunks);

  until (Terminated=true) or (NumberOfCombinedChunks=NumberOfChunksToCombine);
    
end;

procedure TCombineChunksThread.CheckChunksToCombine;
begin

  if ChunksToCombineList.Count>0 then
  begin
    ChunkToCombine:=ChunksToCombineList.Strings[0];
    ChunksToCombineList.Delete(0);
  end
  else
  begin
    ChunkToCombine:='';
    Suspend;
  end;


end;

function TCombineChunksThread.CombineChunk(ChunkIndex:string):boolean;
var OutputFile,InputFile:string;
    BytesRead, BytesWritten: Integer;
    DataArray: array of byte;
    FlagsAndAttributes: DWORD;
    SourceFileHandle,TargetFileHandle: THandle;
    SourceStream,TargetStream: TFileStream;
    SumOfBytesRead,SumOfBytesWritten,SourceFileSize,TargetFileSize:int64;
    BufferSize:integer;
    function ReturnFileSize(const FileName: string):Int64;
    var SearchRec : TSearchRec;
    begin
      if FindFirst(FileName,faAnyFile,SearchRec)=0 then Result:=Int64(SearchRec.FindData.nFileSizeHigh) shl Int64(32)+Int64(SearchREc.FindData.nFileSizeLow)
      else Result:=0;
      FindClose(SearchRec);
    end;
begin

  Result:=false;

  InputFile:=FolderWithChunks+'\'+ChunkIndex+codec_extension;
  OutputFile:=VideoPartFile;

  FlagsAndAttributes:=FILE_ATTRIBUTE_NORMAL;
  SourceFileHandle:=CreateFile(PChar(InputFile), GENERIC_READ, FILE_SHARE_READ, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);
  TargetFileHandle:=CreateFile(PChar(OutputFile), GENERIC_WRITE, FILE_SHARE_WRITE, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);

  if SourceFileHandle=INVALID_HANDLE_VALUE then exit;

  SourceFileSize:=ReturnFileSize(InputFile);
  TargetFileSize:=ReturnFileSize(OutputFile);

  SourceStream:=TFileStream.Create(SourceFileHandle);
  TargetStream:=TFileStream.Create(TargetFileHandle);

  BufferSize:=16777216;       //16 MiB
  SetLength(DataArray,BufferSize);

  SumOfBytesRead:=0;
  SumOfBytesWritten:=0;
    
  try
    TargetStream.Position:=TargetFileSize;
    repeat

      BytesRead:=SourceStream.Read(DataArray[0],BufferSize);
      SumOfBytesRead:=SumOfBytesRead+BytesRead;

      BytesWritten:=TargetStream.Write(DataArray[0],BytesRead);
      SumOfBytesWritten:=SumOfBytesWritten+BytesWritten;

    until (BytesRead=0) or (BytesWritten=0) or (BytesRead<>BytesWritten) or (Terminated=true);

  finally

    SourceStream.Free;
    TargetStream.Free;

    CloseHandle(SourceFileHandle);
    CloseHandle(TargetFileHandle);

    if SumOfBytesWritten=SourceFileSize then Result:=true;
  end;

end;


Parę godzin temu odkryłem że powodem zawieszania jest zwalnianie pamięci w funkcji CombineChunk. Gdy usunę te dwie linijki to program się nie zwiesza i wszystko działa jak należy.

SourceStream.Free; //lub FreeAndNil(SourceStream)
TargetStream.Free;  //lub FreeAndNil(TargetStream)

Może ktoś mi to wytłumaczyć dlaczego sprzątanie po sobie powoduje takie anomalie?
Ps. Używam Delphi7 oraz Windows7 x64 SP1 + najnowsze aktualizacje

0

Nie wiem dokładnie co może być problemem – musiałbym sprawdzić, ale że Delphi 7 nie używam od lat i nie zamierzam, to mogę jedynie trochę ponarzekać na kod który pokazałeś. A pierwszą rzeczą jest to:

Gdy usunę te dwie linijki to program się nie zwiesza i wszystko działa jak należy.

Jeśli usuniesz te dwie linijki to Twój program będzie generował wycieki pamięci. Jeśli dynamicznie zaalokowałeś pamięć (np. poprzez stworzenie instancji klasy) to musisz ją sam zwolnić – koniec kropka. Następna rzecz:

SourceFileHandle:=CreateFile(PChar(InputFile), GENERIC_READ, FILE_SHARE_READ, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);
TargetFileHandle:=CreateFile(PChar(OutputFile), GENERIC_WRITE, FILE_SHARE_WRITE, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);

if SourceFileHandle=INVALID_HANDLE_VALUE then exit;

Tutaj dostaniesz wyciek, jeśli warunek zostanie spełniony – nie zamykasz pliku spod TargetFileHandle.

TargetStream.Position:=TargetFileSize;

Tę linijkę możesz wymienić na TargetStream.Seek. Dalej jest jeszcze gorzej:

finally

    SourceStream.Free;
    TargetStream.Free;

    CloseHandle(SourceFileHandle);
    CloseHandle(TargetFileHandle);

Jeśli zwolnisz oba strumienie, to automatycznie zamkniesz oba pliki, a więc wywołania CloseFile będą operować na nieaktualnych uchwytach. No zdecyduj się – albo rybki, albo akwarium.

Skoro używasz funkcji z systemowego API, to po co łączysz je z wysokopoziomowymi klasami typu TFileStream? Albo używaj samych funkcji systemowych, albo samych wysokopoziomowych klas, w przeciwnym razie robi się bajzel. Z drugiej strony, skoro już używasz klasy TFileStream, to całe Twoje czytanie danych fragmentami nie ma żadnego sensu, bo o ile mnie pamięć nie myli, to metoda TFileStream.CopyFrom implementuje całe tego typu buforowanie.

Jeśli miałbym Ci coś konkretnego doradzić, to widzę dwie opcje.

Pierwsza to użycie wyłącznie klas TFileStream oraz przekopiowanie całej zawartości strumienia wejściowego do wyjściowego za pomocą CopyFrom. Żadnych pierdół i zabawy w buforowanie – kilka linii kodu i wszystko zrobione. Wadą tego rozwiązania nadal będzie problem z zawieszaniem się programu, który to trzeba będzie namierzyć. Choć obstawiam, że wspomniane wcześniej podwójne zamykanie plików jest tego przyczyną.

Druga opcja to skorzystanie wyłącznie z systemowych funkcji, czyli CreateFile oraz ReadFile i WriteFile, a na koniec CloseFile. Co prawda będziesz musiał sam zaimplementować odczyt i zapis buforowany, ale za to odpadnie problem z zawieszaniem programu i wyciekami pamięci.

1

grzebiesz w kontrolkach z pobocznego wątku

0

Dla Delphi 7 to https://github.com/pleriche/FastMM4 w pierwszej kolejności.

0

Jeśli zwolnisz oba strumienie, to automatycznie zamkniesz oba pliki, a więc wywołania CloseFile będą operować na nieaktualnych uchwytach. No zdecyduj się – albo rybki, albo akwarium.

Nie wiedziałem że .free również zwalnia uchwyt i dlatego użyłem CloseHandle jak bozia nakazuje.

Tak czy siak. Zmodyfikowałem funkcję i teraz sprawdzam czy będzie już ok...

function TCombineChunksThread.CombineChunk(ChunkIndex:string):boolean;
var OutputFile,InputFile:string;
    BytesRead, BytesWritten: Integer;
    DataArray: array of byte;
    FlagsAndAttributes: DWORD;
    SourceFileHandle,TargetFileHandle: THandle;
    SourceStream,TargetStream: TFileStream;
    SumOfBytesRead,SumOfBytesWritten,SourceFileSize,TargetFileSize:int64;
    BufferSize:integer;
    function ReturnFileSize(const FileName: string):Int64;
    var SearchRec : TSearchRec;
    begin
      if FindFirst(FileName,faAnyFile,SearchRec)=0 then Result:=Int64(SearchRec.FindData.nFileSizeHigh) shl Int64(32)+Int64(SearchREc.FindData.nFileSizeLow)
      else Result:=0;
      FindClose(SearchRec);
    end;
begin

  Result:=false;

  InputFile:=FolderWithChunks+'\'+ChunkIndex+codec_extension;
  OutputFile:=VideoPartFile;

  FlagsAndAttributes:=FILE_ATTRIBUTE_NORMAL;

  repeat
    SourceFileHandle:=CreateFile(PChar(InputFile), GENERIC_READ, FILE_SHARE_READ, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);
    if SourceFileHandle=INVALID_HANDLE_VALUE then Sleep(250);
  until (Terminated=true) or (SourceFileHandle<>INVALID_HANDLE_VALUE);

  if Terminated=false then
  begin
    repeat
      TargetFileHandle:=CreateFile(PChar(OutputFile), GENERIC_WRITE, FILE_SHARE_WRITE, nil,OPEN_ALWAYS, FlagsAndAttributes, 0);
      if TargetFileHandle=INVALID_HANDLE_VALUE then Sleep(250);
    until (Terminated=true) or (TargetFileHandle<>INVALID_HANDLE_VALUE);
  end;
                                           
  if Terminated=true then
  begin
    if SourceFileHandle<>INVALID_HANDLE_VALUE then CloseHandle(SourceFileHandle);
    if TargetFileHandle<>INVALID_HANDLE_VALUE then CloseHandle(TargetFileHandle);
  end
  else
  begin
    SourceFileSize:=ReturnFileSize(InputFile);
    TargetFileSize:=ReturnFileSize(OutputFile);

    SourceStream:=TFileStream.Create(SourceFileHandle);
    TargetStream:=TFileStream.Create(TargetFileHandle);

    BufferSize:=16777216;       //16 MiB
    SetLength(DataArray,BufferSize);

    SumOfBytesRead:=0;
    SumOfBytesWritten:=0;
    
    try
      TargetStream.Position:=TargetFileSize;
      repeat

        BytesRead:=SourceStream.Read(DataArray[0],BufferSize);
        SumOfBytesRead:=SumOfBytesRead+BytesRead;

        BytesWritten:=TargetStream.Write(DataArray[0],BytesRead);
        SumOfBytesWritten:=SumOfBytesWritten+BytesWritten;

      until (BytesRead=0) or (BytesWritten=0) or (BytesRead<>BytesWritten) or (Terminated=true);

    finally

      CloseHandle(SourceFileHandle);
      CloseHandle(TargetFileHandle);

      if Assigned(SourceStream)=true then SourceStream.Free;
      if Assigned(TargetStream)=true then TargetStream.Free;

      if SumOfBytesWritten=SourceFileSize then Result:=true;
    end;
  end;

end;
0

Pisałem Ci żebyś wywalił ten kod buforujący kopiowanie, bo nie ma on żadnego sensu, a tylko wydłuża całość. I nadal robisz dokładnie ten sam błąd co wcześniej – jednocześnie używasz funkcji z systemowego API i wysokopoziomowych klas, które wykluczają się wzajemnie.

Poza tym zlituj się i wywal te porównania wartości logicznych do True… Nie dość że niepotrzebnie wydłużasz kod, to jeszcze w niektórych przypadkach dodajesz nawiasy, co jeszcze bardziej komplikuje kod i utrudnia jego analizę.

0

Ale tak mi się podoba. Czy ja muszę pisać tak jak ty? Co to? Polski StackOverflow? Kopiowanie zrobiłem po swojemu bo chcę mieć większą kontrolę (możliwość ustawienia własnego rozmiaru bufora czy odpowiednie zliczanie bajtów). Czy TFileStream.CopyFrom również daje mi taką możliwość? Jedyny kluczowy błąd jaki mogłem faktycznie popełnić to zwalnianie pamięci przed zwalnianiem uchwytu. To by tłumaczyło dlaczego po moim desperackim usunięciu SourceStream.Free i TargetStream.Free kod w wątku się nie wieszał. Zawieszenie było najprawdopodobniej spowodowane przez CloseHandle(SourceFileHandle) i CloseHandle(TargetFileHandle) ,które odnosiły się do niewłaściwego już uchwytu. Tajemnicą pozostanie tylko dlaczego funkcja nie wywalała się już za pierwszym przebiegiem?

0
Atak_Snajpera napisał(a):

Ale tak mi się podoba. Czy ja muszę pisać tak jak ty? Co to? Polski StackOverflow?

Ale mi się nie podoba, a to ja analizuję ten kod i szukam bugów, których w nim nasadziłeś. I utrudniasz mi robotę poprzez dowalanie instrukcji, które niczego prócz obfuskacji nie dają. Więc jak już prosisz kogokolwiek o pomoc i wrzucasz kod na forum to przynajmniej ogarnij go tak, aby okroić problem do minimum.

Kopiowanie zrobiłem po swojemu bo chcę mieć większą kontrolę (możliwość ustawienia własnego rozmiaru bufora czy odpowiednie zliczanie bajtów). Czy TFileStream.CopyFrom również daje mi taką możliwość?

A czy Twój kod pokazuje cokolwiek związanego z kontrolą kopiowania? Robisz coś w tej pętli prócz przepisywania danych z jednego pliku do drugiego? Nie. Nie wiesz nawet co się sypie w tym programie i zamiast skrócić ten kod do minimum aby łatwiej było namierzyć bugi, to tylko dokładasz kolejnych wymyślonych na nowo kół, które tylko zwiększają pulę możliwych problemów.

Jedyny kluczowy błąd jaki mogłem faktycznie popełnić to zwalnianie pamięci przed zwalnianiem uchwytu.

Kluczowym błędem jest mieszanie klasy TFileStream z funkcjami systemowymi, bo to komplikuje kod i trzeba cudować zamiast skupić się na realnym zadaniu, czyli na otwieraniu plików i kopiowaniu danych.

Używasz CreateFile bo TFileStream rzuca wyjątki w konstruktorze, jeśli pojawi się problem. Idź za ciosem i użyj SetFilePointer oraz w pętli ReadFile i WriteFile, na koniec dupnij CloseFile i z głowy. Będziesz miał pełen wachlarz możliwości, kod trzymający się kupy i brak problemów z wyciekami czy zawieszaniem się wątku.

1 użytkowników online, w tym zalogowanych: 0, gości: 1