+[TC8.50b9] 'Not enough memory' causes wrong compare result

[Izibia]

If I compare two very large but identical files by "Compare by Content", I get an error message:

Not enough memory, search aborted.
The two files are DIFFERENT!

Similarly if I compare that 2 identical files by content via "Synchronize Dirs", and I get an error message

Out of system resources
Windows XP SP3 5.1 (Build 2600)

Please report this error to the Author, with a description
of what you were doing when this error occurred!

Delphi exception: EOutOfResources
Stack trace:
....

then the files are marked as different.

[ghisler(Author)]

Unfortunately the only thing you can do to avoid the first one is to use Total Commander 64-bit on a 64-bit version of Windows. Why? 32-bit TC only has 2 GB of virtual memory available, and it may already be very fragmented when trying to map the two files into memory.

There is nothing you can do about 2.

[Hacker]

Christian,
Well but why does TC say the files are different if they are identical? Can it not just say "out of memory, can't really say if they are identical or not" instead of saying they are different (which is not true, according to the OP)?

Roman

[krasusczak]

Agree, or TC can also say something like this: until this moment (x%) file are identical/different, but no memory to test rest of the file.

[MVV]

I think TC should map not whole files but parts of them (e.g. 64MB blocks) and then move window so it wouldn't require such ammount of memory. When difference is found, TC may load entire files for compare tool in order to show differences (if it is too hard to use window with limited size).

[ghisler(Author)]

2Hacker
When TC shows this, it has successfully compared the two files by by byte (binary comparison) and found a difference. Only then it tries to map the two files into memory and show the differences. But TC already knows then that the two are different, it just can't show the differences in the editor.

[Izibia]

When TC shows this, it has successfully compared the two files by by byte (binary comparison) and found a difference.

Then we have another problem:
just after TC showed the error message, fc.exe reported these two files as identical. Then I cleared RAM (I ended most processes, no system restart) and ran TC again, and it reported the files to be identical.

edit:
maybe this is a related issue?
http://ghisler.ch/board/viewtopic.php?t=38338

I think TC should map not whole files but parts of them (e.g. 64MB blocks) and then move window so it wouldn't require such ammount of memory. When difference is found, TC may load entire files for compare tool in order to show differences (if it is too hard to use window with limited size).

This is slightly off-topic here, but ...
I have a suspicion Christian doesn't want to involve more deep into the issue, because there are more problems about this issue, for example:
http://ghisler.ch/board/viewtopic.php?p=274989#274989

[ghisler(Author)]

Then we have another problem:
just after TC showed the error message, fc.exe reported these two files as identical. Then I cleared RAM (I ended most processes, no system restart) and ran TC again, and it reported the files to be identical.

No, then YOU have a problem - it means that at least one bit was flipped when comparing the two files. It could be bad RAM, or a read error due to a bad harddisk cable. TC only tries to show the compare dialog when it finds a binary difference (not when there is a read error or out of memory during the first compare step).

maybe this is a related issue?
http://ghisler.ch/board/viewtopic.php?t=38338

It's possible that he has a bad RAM chip too - this also results in CRC errors when creating huge ZIP files. Unfortunately he didn't reply to my last question, and no one else was willing to test the problem either.

This is slightly off-topic here, but ...
I have a suspicion Christian doesn't want to involve more deep into the issue, because there are more problems about this issue, for example:
http://ghisler.ch/board/viewtopic.php?p=274989#274989

Please don't make such unfounded assumptions. I have replied to all reports, there is nothing to hide.

[petermad]

Then I cleared RAM (I ended most processes, no system restart) and ran TC again, and it reported the files to be identical.

No, then YOU have a problem

If the two files ARE identical and TC reports that they are identical, I don't see any problem

[ghisler(Author)]

The problem is that they are identical, but TC reports that they are different.

I found one case where TC shows that they are different although this isn't certain: When ReadFile fails for one of the files during the initial compare step. This can only happen in 2 cases:
1. The file is locked in the middle, e.g. a database file
or
2. ReadFile failed for some more serious reason, e.g. a disk read error.

I have now added code for this case, so TC will show in which file reading failed.

[ghisler(Author)]

Can anyone try this with beta 12, please?

[MarcinW]

I performed some tests with Beta 12 and got interesting results. Below is a small test program - it creates two identical files (C:\dir1\TEST.txt and C:\dir2\TEST.txt) and locks whole C:\dir1\TEST.txt file (with LockFile API).


Test 1:
1) Launch "Test.exe /test1" - this will create test files (with size > 1MB), lock one of them and display a message box - don't close this message box, but go (with TC) to C:\dir1 in one panel and C:\dir2 in another panel.
2a) Use "File -> Compare By Content" to compare TEST.txt in both panels.
Expected: Message box "The two files have the same content!".
Currently: "Compare contents" window appears, but shows "0 differences found".
2b) Select "Commands -> Synchronize Dirs", check "by content" option and press "Compare" button.
Expected: files are shown as identical.
Currently: files are shown as different.


Test 2:
1) Launch "Test.exe /test2" - this will create test files (with size < 1MB), ...
2a) Use "File -> Compare By Content" to compare TEST.txt in both panels.
Expected: as in Test 1.
Currently: message box: "Access denied on file c:\dir1\TEST.txt!".
2b) Select "Commands -> Synchronize Dirs", check "by content" option and press "Compare" button.
Expected: as in Test 1.
Currently: as in Test 1.


Conclusions:
- The window, that shows results of comparing, can read locked files of size > 1MB, but can't < 1MB. Reading files larger than 1MB is possible, because a file mapping is used for such files - and, as MSDN says, "locking a region of a file does not prevent reading from a mapped file view".
- The function, that simply checks if files are identical or not, doesn't use file mappings at all, so it always fails when comparing locked files.


To solve all these problems:
Create a global wrapper for ReadFile function. In all cases, when ReadFile fails with error code 33 (ERROR_LOCK_VIOLATION), create a file mapping - not for the whole file, but only for the area being currently read - and get file contents from a view of the mapped file.


Regards

Code: Select all

program Test;
uses
  Windows, SysUtils;
const
  FileName1 = 'C:\dir1\TEST.txt';
  FileName2 = 'C:\dir2\TEST.txt';
var
  FileBytes : Integer;
  F : TextFile;
  S : string;
  I : Integer;
  FH : THandle;
begin
  try
    {1. Check input parameter}
    S:=LowerCase(ParamStr(1));
    if S = '/test1' then
      FileBytes:=$100000+$10
    else
    if S = '/test2' then
      FileBytes:=$100000-$10
    else
      raise Exception.Create('Use parameter /test1 or /test2.');

    try
      {2. Initialize test}
      CreateDir(ExtractFilePath(FileName1));
      CreateDir(ExtractFilePath(FileName2));

      S:='0123456789abcd';

      AssignFile(F,FileName1);
      Rewrite(F);
      for I:=1 to FileBytes div (Length(S)+Length(#13#10)) do
        Writeln(F,S);
      CloseFile(F);

      AssignFile(F,FileName2);
      Rewrite(F);
      for I:=1 to FileBytes div (Length(S)+Length(#13#10)) do
        Writeln(F,S);
      CloseFile(F);

      {3. Perform test}
      FH:=CreateFile(PChar(FileName1),GENERIC_READ or GENERIC_WRITE,FILE_SHARE_READ,nil,OPEN_EXISTING,0,0);
      if FH = INVALID_HANDLE_VALUE then
        raise Exception.Create('CreateFile failed.');
      try
        if not LockFile(FH,0,0,High(Integer),0) then
          raise Exception.Create('LockFile failed.');
        MessageBox(0,PChar('File '+FileName1+'  is locked now - perform the TC test!'),'Test',MB_ICONINFORMATION);
      finally
        CloseHandle(FH);
      end;
    finally
      {4. Finalize test}
      DeleteFile(FileName1);
      DeleteFile(FileName2);
      RemoveDir(ExtractFilePath(FileName1));
      RemoveDir(ExtractFilePath(FileName2));
    end;
  except
    on E:Exception do
      MessageBox(0,PChar(E.ClassName+':'#13#10+E.Message),'Exception',MB_ICONSTOP);
  end;
end.

[ghisler(Author)]

Sorry, I don't want to change the way how files are loaded/mapped. While mapping has some advantages on locked files, it also has some big disadvantages: For example, users may want to use an external editor to edit (small) text files, which cannot be done when mapping files. This is one of the reasons why I'm using file mapping only when memory becomes a concern.

[MarcinW]

When TC creates a read-only file mapping (and keeps it open!), there are two possibilities:

1) Some other application (like Notepad) may want to open the file and write to it - this may fail.
2) Some other application (like database engine) opened the file before creating the file mapping by TC - this application may write to file without any problems.

Assuming that TC keeps the file mapping open (and only then), case 1 may be problematic.


Please note that I propose a different solution. This solution:
a) doesn't create a file mapping and keep it open - it only creates a file mapping for a very short period of time (milliseconds or microseconds), so the user will not be able to switch to another application and try to overwrite the file,
b) it creates a file mapping only when ReadFile fails due to a locked file region - locking region of file is not what applications like Notepad do, and even if - an interference is impossible: if some application keeps a region of file locked, it surely has this file open - this is the case 2 above, which is not problematic at all.


Let's assume that TC tries to read file contents of size 4kB. The algorithm should be:
1) Call ReadFile for this 4kB. If it succeeds, you have 4kB of file contents in the buffer. If fails, and only if error code is ERROR_LOCK_VIOLATION:
2) Create a file mapping of size 4kB,
3) Map view of file,
4) Copy contents of this view to the 4kB buffer,
5) Immediately after this unmap view of file,
6) Delete the file mapping.

This approach has much less potential side effects, than the current implementation of comparison for > 1MB files (TC creates file mappings and keeps them open all the time when displaying the differences), so I think it's worth considering.


Regards

[ghisler(Author)]

So you mean that TC should only create a file mapping for the initial compare phase (outside of the compare tool)?