Lister don't use 'FILE_SHARE_DELETE' to open Files

Please report only one bug per message!

Moderators: sheep, Hacker, Stefan2, white

Post Reply
HAL 9000
Senior Member
Senior Member
Posts: 384
Joined: 2007-09-10, 13:05 UTC

Lister don't use 'FILE_SHARE_DELETE' to open Files

Post by *HAL 9000 » 2012-09-20, 14:01 UTC

Lister don't use 'FILE_SHARE_DELETE' to open Files

Well that is not really critical and may only in rare circumstances cause that the TC-Lister fails.

However I encountered such a scenario when messing around with the Flashplayer.

It'll buffer files in
c:\Temp\acro_rd_dir\fla2281.tmp
'forgot' to set the FILE_SHARE_READ flag on CreateFile and additional set the flag FILE_FLAG_DELETE_ON_CLOSE 0x04000000
I fixed the FILE_SHARE_READ-'bug' to be able to copy or drag fla2281.tmp into VLC just as in old times.

Code: Select all

Patch for
C:\Windows\System32\Macromed\Flash
NPSWF32_11_4_402_265.dll

Search 
  68 00 00 00 14 6A 02 6A 00 68 00 00 00 C0
Replace with
  68 00 00 00 14 6A 02 6A 01
                          ^^-FILE_SHARE_READ

Comments
  68 00000014     PUSH    14000000   ;FILE_FLAG_DELETE_ON_CLOSE 
  6A 02           PUSH    2          ;CREATE_ALWAYS
  6A 00           PUSH    0 <-       ;NO 0x01 FILE_SHARE_READ !!!
  68 000000C0     PUSH    C0000000   ;GENERIC_READ|GENERIC_WRITE
  
  
  0CDD9CC8  |FileName = "\\?\c:\Temp\acro_rd_dir\fla67CF.tmp"
  C0000000  |Access = GENERIC_READ|GENERIC_WRITE
  00000001  |ShareMode = FILE_SHARE_READ
  00000000  |pSecurity = NULL
  00000002  |Mode = CREATE_ALWAYS
  14000000  |Attributes = RANDOM_ACCESS|DELETE_ON_CLOSE
  00000000  \hTemplateFile = NULL
Now copying 'fla2281.tmp' works without error :) however TC-Lister and VLC still don't open 'fla2281.tmp'.
Digging deeper into this reveals that if a file was created with DELETE_ON_CLOSE(and of course with at least FILE_SHARE_READ) and some other application also opens this files it needs FILE_SHARE_DELETE in ShareMode or opening will fail.

So please add the 'FILE_SHARE_DELETE' flag to the ShareMode parameter in CreateFile for opening files in the TC.

Well I did this already in Totalcmd.exe like this
before:

Code: Select all

...
Case 0,4
sharemode:=3 ;FILE_SHARE_WRITE | FILE_SHARE_READ
Case 1
sharemode:=0
Case 2
sharemode:=1 ;FILE_SHARE_READ
Case 3
sharemode:=2 ;FILE_SHARE_WRITE
...
after:

Code: Select all

...Case 0,4
sharemode:=7 ;FILE_SHARE_DELETE | FILE_SHARE_WRITE | FILE_SHARE_READ
Case 1
sharemode:=0
Case 2
sharemode:=5 ;FILE_SHARE_DELETE | FILE_SHARE_READ
Case 3
sharemode:=6 ;FILE_SHARE_DELETE | FILE_SHARE_WRITE
...
and it works fine.
(..well I know the TC selfchecks - 'temporally' disabled this overhead for testing and kept it off for better performance :P )

_______________________________________________________

Reference for the Kernel32.CreateFile API-Function:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa363858%28v=vs.85%29.aspx


That's what the MSDN tells about
FILE_FLAG_DELETE_ON_CLOSE 0x04000000
The file is to be deleted immediately after all of its handles are closed, which includes the specified handle and any other open or duplicated handles.

If there are existing open handles to a file, the call fails unless they were all opened with the FILE_SHARE_DELETE share mode.

Subsequent open requests for the file fail, unless the FILE_SHARE_DELETE share mode is specified.
...and about
FILE_SHARE_READ 0x00000001
FILE_SHARE_WRITE 0x00000002
FILE_SHARE_DELETE 0x00000004

Enables subsequent open operations on a file or device to request delete access.

Otherwise, other processes cannot open the file or device if they request delete access.

If this flag is not specified, but the file or device has been opened for delete access, the function fails.

Note Delete access allows both delete and rename operations.

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2012-09-24, 00:33 UTC

Hmmm, after adding FILE_SHARE_DELETE flag when opening files in TC, other applications will not be able to open files, which are being opened by TC (because most of applications don't use FILE_SHARE_DELETE when opening files).

For example, after opening a file with Lister, we won't be able to view this file with hex editor...

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 38408
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2012-09-24, 14:09 UTC

Thanks for your interesting research! Maybe I should try FILE_SHARE_DELETE only if opening the file without the flag fails?
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2012-09-24, 21:53 UTC

Calling CreateFile again, with FILE_SHARE_DELETE flag, only after CreateFile fails with ERROR_SHARING_VIOLATION would be probably a good idea (no difference for other, "normal" programs).

HAL 9000
Senior Member
Senior Member
Posts: 384
Joined: 2007-09-10, 13:05 UTC

Post by *HAL 9000 » 2012-09-25, 23:04 UTC

MarcinW wrote:Hmmm, after adding FILE_SHARE_DELETE flag when opening files in TC, other applications will not be able to open files, which are being opened by TC (because most of applications don't use FILE_SHARE_DELETE when opening files).

For example, after opening a file with Lister, we won't be able to view this file with hex editor...
That's not correct. Seem ya mixed up FILE_FLAG_DELETE_ON_CLOSE and FILE_SHARE_DELETE.
Opening or creating a file with the FILE_SHARE_DELETE like this:

000A7248 |FileName = "c:\Temp\1\speedpwn-0.4\fb_vbs\liesmich.txt"
80000000 |Access = GENERIC_READ
00000007 |ShareMode = FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE
00000000 |pSecurity = NULL
00000003 |Mode = OPEN_EXISTING
00000000 |Attributes = 0
00000000 \hTemplateFile = NULL

Does no harm or opposes any constrains to other programs that opens the file as well.

Only if you create/open a new file with FILE_FLAG_DELETE_ON_CLOSE it would be like that other app need open the file with FILE_SHARE_DELETE to get access, beside the devastating fact that is the Lister would close this file it'll be done/delete. :twisted:

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 38408
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2012-09-27, 13:37 UTC

Isn't there a risk that when a file is opened with FILE_SHARE_DELETE flag, it could be deleted by the other program while it's still open in TC? What would happen then when ReadFile is called and the file is already gone?
Author of Total Commander
http://www.ghisler.com

HAL 9000
Senior Member
Senior Member
Posts: 384
Joined: 2007-09-10, 13:05 UTC

Post by *HAL 9000 » 2012-09-30, 12:01 UTC

Well yes that's the risk the app should care for and be prepare(error handler /exception handler).
ReadFile might fail with INVALID_HANDLE error or something and return nothing(<-haven't tested yet what happens exactly)

That is probably the reason why you need explicitly specify this flag to get the file open and when doing so you need to care for that your file handle might be close by the OS without any notice.

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2012-10-01, 01:56 UTC

000A7248 |FileName = "c:\Temp\1\speedpwn-0.4\fb_vbs\liesmich.txt"
80000000 |Access = GENERIC_READ
00000007 |ShareMode = FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE
00000000 |pSecurity = NULL
00000003 |Mode = OPEN_EXISTING
00000000 |Attributes = 0
00000000 \hTemplateFile = NULL
HAL 9000, you are right.
Isn't there a risk that when a file is opened with FILE_SHARE_DELETE flag, it could be deleted by the other program while it's still open in TC? What would happen then when ReadFile is called and the file is already gone?
File can be deleted only after last handle to the file is closed. When TC opens file successfully, it can safely read/write this file. Operating system guarantees, that the file handle will be valid until closed by TC (but errors may still occur, for example due to CD/DVD removing, hardware failure etc.). Simple test program (with no error handling):

Code: Select all

uses
  Windows, SysUtils;
var
  H1 : THandle;
  H2 : THandle;
  Buf : Byte;
  Written : DWORD;
begin
  H1:=CreateFile('test.bin',GENERIC_WRITE,FILE_SHARE_READ or FILE_SHARE_WRITE or FILE_SHARE_DELETE,nil,OPEN_ALWAYS,FILE_FLAG_DELETE_ON_CLOSE,0);
  if H1 = INVALID_HANDLE_VALUE then
    Exit;
  WriteFile(H1,Buf,1,Written,nil);

  H2:=CreateFile('test.bin',GENERIC_READ,FILE_SHARE_READ or FILE_SHARE_WRITE or FILE_SHARE_DELETE,nil,OPEN_EXISTING,0,0);
  if H2 = INVALID_HANDLE_VALUE then
    Exit;

  CloseHandle(H1);
  MessageBox(0,PChar('File size is '+IntToStr(GetFileSize(H2,nil))),'Test',MB_ICONINFORMATION);
  CloseHandle(H2);
end.
As we can see, file 'test.bin' is being deleted by operating system after closing handle H2, not H1.

Note: as with normal files, file contents may change in the background - other processes can for example truncate the file, so read operation may fail unexpectedly.

HAL 9000
Senior Member
Senior Member
Posts: 384
Joined: 2007-09-10, 13:05 UTC

Post by *HAL 9000 » 2012-10-01, 21:40 UTC

Nice.
Nice example MarcinW.
So the speculations I wrote here is not correct:
HAL 9000 wrote:Well yes that's the risk the app should care for and be prepare(error handler /exception handler).
ReadFile might fail with INVALID_HANDLE error or something and return nothing(<-haven't tested yet what happens exactly)
It's even better. Opening such a file(created with FILE_FLAG_DELETE_ON_CLOSE) in TC-Lister(with FILE_SHARE_DELETE) will preserve it from being deleted, until you close it in the Lister.

No hassle to care for any file handle that might get invalid or additional error handlers. :D

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 38408
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2012-10-04, 13:29 UTC

No, actually not - Lister opens a file only temporarily, reads about 64k of data around the current position, and then closes the file.
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2012-10-11, 18:37 UTC

So the code for reading this 64kB in Lister could be like this:

Code: Select all

uses 
  Windows, SysUtils;
const
  FileName = 'test.bin';
var
  H : THandle;
  Buffer : array[0..65536] of Byte;
  Read : DWORD;
begin
  H:=CreateFile(FileName,GENERIC_READ,FILE_SHARE_READ or FILE_SHARE_WRITE or FILE_SHARE_DELETE,nil,OPEN_EXISTING,FILE_FLAG_SEQUENTIAL_SCAN,0);
  if H <> INVALID_HANDLE_VALUE then
  try
    ReadFile(H,Buffer[0],SizeOf(Buffer),Read,nil);
  finally
    CloseHandle(H);
  end;
end.
When the user scrolls text in Lister, file operations should also use FILE_SHARE_DELETE flag, similarly as above.

Regards

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 38408
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2012-10-15, 13:32 UTC

Yes, exactly.
Author of Total Commander
http://www.ghisler.com

User avatar
MarcinW
Power Member
Power Member
Posts: 852
Joined: 2012-01-23, 15:58 UTC
Location: Poland

Post by *MarcinW » 2013-10-04, 00:57 UTC

27.01.13 Added: Lister: Open file with flag FILE_SHARE_DELETE on NT-based systems if normal open fails (32/64)
As I just checked with TC 8.50 beta 5 (32-bit), the problem is currently solved.

User avatar
ghisler(Author)
Site Admin
Site Admin
Posts: 38408
Joined: 2003-02-04, 09:46 UTC
Location: Switzerland
Contact:

Post by *ghisler(Author) » 2013-10-04, 15:23 UTC

Nice to hear that, thanks for trying it!
Author of Total Commander
http://www.ghisler.com

Post Reply