Potential TList leak in DiskRestoreFromOperations in UBlockChain.pas

Started by Skybuck, January 27, 2022, 01:29:47 PM

Previous topic - Next topic

Skybuck

See *** for problems.

Recommendation:

    Almost never use exit, if you must exit use goto and labels to jump to proper exit code/cleanup code.
    For functions set result to False, when exiting successfully at end of function set result to true.
    Skip over false conditions with if statements
    Always clean up/free objects especially list at end of procedures.

See code at line 946 in UBlockChain.pas

procedure TPCBank.DiskRestoreFromOperations(max_block : Int64; restoreProgressNotify : TProgressNotify = Nil);
Var
  errors: String;
  n : Int64;
  tc, LStartProcessTC : TTickCount;
  LBlocks : TList<TPCOperationsComp>; // *** LIST OF OBJECTS ***
  LTmpPCOperationsComp : TPCOperationsComp;
  i,j, LProgressBlock, LProgressEndBlock, LOpsInBlocks : Integer;
  LSafeboxTransaction : TPCSafeBoxTransaction;
  LTempSafebox : TPCSafeBox;
begin
  if FIsRestoringFromFile then begin
    TLog.NewLog(lterror,Classname,'Is Restoring!!!');
    raise Exception.Create('Is restoring!');
  end;
  tc := TPlatform.GetTickCount;
  LStartProcessTC := tc;
  TPCThread.ProtectEnterCriticalSection(Self,FBankLock);
  try
    FIsRestoringFromFile := true;
    try
      Clear;
      Storage.Initialize;
      If (max_block<Storage.LastBlock) or (Storage.LastBlock<0) then n := max_block
      else n := Storage.LastBlock;

      RestoreBank(n,Orphan,restoreProgressNotify);
      // Restore last blockchain
      if (BlocksCount>0) And (SafeBox.CurrentProtocol=CT_PROTOCOL_1) then begin
        if Not Storage.LoadBlockChainBlock(FLastBlockCache,BlocksCount-1) then begin
          NewLog(nil,lterror,'Cannot find blockchain '+inttostr(BlocksCount-1)+' so cannot accept bank current block '+inttostr(BlocksCount));
          Clear;
        end else begin
          FLastOperationBlock := FLastBlockCache.OperationBlock;
        end;
      end;
      If SafeBox.BlocksCount>0 then FLastOperationBlock := SafeBox.GetBlockInfo(SafeBox.BlocksCount-1)
      else begin
        FLastOperationBlock := TPCOperationsComp.GetFirstBlock;
        FLastOperationBlock.initial_safe_box_hash := TPCSafeBox.InitialSafeboxHash; // Genesis hash
      end;

      NewLog(Nil, ltinfo,'Start restoring from disk operations (Max '+inttostr(max_block)+') BlockCount: '+inttostr(BlocksCount)+' Orphan: ' +Orphan);
      LBlocks := TList<TPCOperationsComp>.Create; // *** LIST CREATED ***
      try
        LProgressBlock := 0;
        LProgressEndBlock := Storage.LastBlock - BlocksCount;
        while ((BlocksCount<=max_block)) do begin
          i := BlocksCount;
          j := i + 99;
          // Load a batch of TPCOperationsComp;
          try
            LOpsInBlocks := 0;
            while ((i<=max_block) and (i<=j)) do begin
              if Storage.BlockExists(i) then begin
                LTmpPCOperationsComp := TPCOperationsComp.Create(Self);
                if Storage.LoadBlockChainBlock(LTmpPCOperationsComp,i) then begin
                  LBlocks.Add(LTmpPCOperationsComp); // *** OBJECT ADDED TO LIST ***
                  inc(LOpsInBlocks, LTmpPCOperationsComp.Count);
                  inc(i);
                end else begin
                  LTmpPCOperationsComp.Free;
                  Break;
                end;
              end else Break;
            end;

            if (LBlocks.Count=0) then Exit; // *** EXIT, LIST NOT FREEED ?! ***

            if Assigned(restoreProgressNotify) then begin
              restoreProgressNotify(Self,Format('Reading blocks from %d to %d with %d operations',[BlocksCount,i,LOpsInBlocks]),0,0);
            end;

            TPCOperationsBlockValidator.MultiThreadValidateOperationsBlock(LBlocks);
            LSafeboxTransaction := TPCSafeBoxTransaction.Create(SafeBox);
            try
              TPCOperationsSignatureValidator.MultiThreadPreValidateSignatures(LSafeboxTransaction,LBlocks,restoreProgressNotify);
            finally
              LSafeboxTransaction.Free;
            end;

            for i := 0 to LBlocks.Count-1 do begin
              inc(LProgressBlock);
              SetLength(errors,0);
              if Not AddNewBlockChainBlock(LBlocks,0,errors) then begin
                NewLog(LBlocks, lterror,'Error restoring block: ' + Inttostr(BlocksCount)+ ' Errors: ' + errors);
                Storage.DeleteBlockChainBlocks(BlocksCount);
                Exit; // *** LIST NOT FREEED ***
              end else begin
                // To prevent continuous saving...
                if ((BlocksCount+(CT_BankToDiskEveryNBlocks*2)) >= Storage.LastBlock ) or
                   ((BlocksCount MOD (CT_BankToDiskEveryNBlocks*10))=0) then begin
                  SaveBank(False);
                end;
                if (Assigned(restoreProgressNotify)) And (TPlatform.GetElapsedMilliseconds(tc)>1000) then begin
                  tc := TPlatform.GetTickCount;
                  restoreProgressNotify(Self,Format('Reading blockchain block %d/%d',[LBlocks.OperationBlock.block,Storage.LastBlock]),LProgressBlock,LProgressEndBlock);
                end;
              end;
            end;
          finally
            // Free blocks
            for i := 0 to LBlocks.Count-1 do begin
              LBlocks.Free;
            end;
            LBlocks.Clear;
          end;

        end; // while

      finally
        LBlocks.Free;
        NewLog(Nil, ltinfo,'End restoring from disk operations (Max '+inttostr(max_block)+') Orphan: ' + Orphan+' Restored '+Inttostr(BlocksCount)+' blocks in '+IntToStr(TPlatform.GetElapsedMilliseconds(LStartProcessTC))+' milliseconds');
      end;

    finally
      FIsRestoringFromFile := False;
      for i := 0 to FNotifyList.Count - 1 do begin
        TPCBankNotify(FNotifyList.Items).NotifyNewBlock;
      end;
    end;
    {$IFDEF USE_ABSTRACTMEM}
    SafeBox.PCAbstractMem.FlushCache;
    {$ENDIF}
  finally
    FBankLock.Release;
  end;
end;