Incompatibility in Kernel32

Hi group,

As I was trying write a fix for msi.dll, I stumbled upon an incompatibility in the kernel.

If you open an existing file, FILE_SHARE_DELETE doesn't seem to work in Wine. In Windows, it does.

Attached is a minimal test case.

I feel hesitation towards starting to hack in kernel32 myself. Though this bug seems to be blocking me to implement msi.dll completely compatible with the MS implementation... Shall I file a bug report?

Regards,
Robert

[Dmitry Timoshkov]

"[e-mail]"  wrote:
> I feel hesitation towards starting to hack in kernel32 myself. Though this
> bug seems to be blocking me to implement msi.dll completely compatible with
> the MS implementation...

You shouldn't look how native msi.dll behaves internally.
> Shall I file a bug report?

Try to add a test case to dlls/kenel32/tests/file.c,test_file_sharing().
> HANDLE h2 = CreateFileA(filename, GENERIC_READ, FILE_SHARE_DELETE | FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0);
>   assert (h2 != 0);
>
> if (!DeleteFile(filename)) {
>     r = GetLastError();
>     fprintf(stderr, "This only happens in Wine. I got error: %d\n", r);
>     assert(0);  
>   }

Is there any reason that you call DeleteFile() on a still being opened file?

[Hans Leidekker]

On Thu, 2012-06-21 at 16:03 +0900, Dmitry Timoshkov wrote:
> > Shall I file a bug report?
>
> Try to add a test case to dlls/kenel32/tests/file.c,test_file_sharing().
> > HANDLE h2 = CreateFileA(filename, GENERIC_READ, FILE_SHARE_DELETE | FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0);
> >   assert (h2 != 0);
> >
> > if (!DeleteFile(filename)) {
> >     r = GetLastError();
> >     fprintf(stderr, "This only happens in Wine. I got error: %d\n", r);
> >     assert(0);  
> >   }
>
> Is there any reason that you call DeleteFile() on a still being opened file?

That's the whole point of this test. The observed behavior is that you can pass a
filename to MsiRecordSetStream, successfully delete the file, and keep on using the
stream.

We currently implement MsiRecordSetStream by creating an in-memory copy of the
stream. As Robert noticed, this will fail if the stream is too large.

The idea is to move to an implementation on top of a file handle, but it should
still allow DeleteFile to succeed while the file is in use.

[Dmitry Timoshkov]

Hans Leidekker  wrote:
> > Is there any reason that you call DeleteFile() on a still being opened file?
>
> That's the whole point of this test. The observed behavior is that you can pass a
> filename to MsiRecordSetStream, successfully delete the file, and keep on using the
> stream.
>
> We currently implement MsiRecordSetStream by creating an in-memory copy of the
> stream. As Robert noticed, this will fail if the stream is too large.
>
> The idea is to move to an implementation on top of a file handle, but it should
> still allow DeleteFile to succeed while the file is in use.

Thanks, I see. Removing GENERIC_WRITE from flags passed to NtCreateFile by
DeleteFile makes the test I just sent for that behaviour pass (and that's
most like a correct fix), but that leads to some other test failures.
This needs further investigation.

[Ben Peddell]

On 21/06/2012 7:11 PM, Dmitry Timoshkov wrote:
> Hans Leidekker  wrote:
> > > Is there any reason that you call DeleteFile() on a still being opened file?
> >
> > That's the whole point of this test. The observed behavior is that you can pass a
> > filename to MsiRecordSetStream, successfully delete the file, and keep on using the
> > stream.
> >
> > We currently implement MsiRecordSetStream by creating an in-memory copy of the
> > stream. As Robert noticed, this will fail if the stream is too large.
> >
> > The idea is to move to an implementation on top of a file handle, but it should
> > still allow DeleteFile to succeed while the file is in use.
>
> Thanks, I see. Removing GENERIC_WRITE from flags passed to NtCreateFile by
> DeleteFile makes the test I just sent for that behaviour pass (and that's
> most like a correct fix), but that leads to some other test failures.
> This needs further investigation.

server/fd.c:open_fd() doesn't check that the read-only bit is unset
(i.e. the file is writable) when the DELETE access flag is set.  Thus
DeleteFile() succeeding when the tests expect it to fail.

> > Is there any reason that you call DeleteFile() on a still being opened file?
>
> That's the whole point of this test. The observed behavior is that you can pass a
> filename to MsiRecordSetStream, successfully delete the file, and keep on using the
> stream.

I think DeleteFile on a opened file is officially allowed in Windows. Although the MS documentation is vague:

<QUOTE>
The DeleteFile function fails if an application attempts to delete a file that is open for normal I/O or as a memory-mapped file.

The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed. Subsequent calls to CreateFile to open the file fail with ERROR_ACCESS_DENIED.
</QUOTE>

These statements seem to be a bit contradictive to me, so that's why I though I'd write a test to see how Windows actually behaves, and then I found this incompatibility.

Regards,
Robert

[Ben Peddell]

On 22/06/2012 7:23 PM, [e-mail] wrote:
> > > Is there any reason that you call DeleteFile() on a still being opened file?
> >
> > That's the whole point of this test. The observed behavior is that you can pass a
> > filename to MsiRecordSetStream, successfully delete the file, and keep on using the
> > stream.
>
> I think DeleteFile on a opened file is officially allowed in Windows. Although the MS documentation is vague:
>
> <QUOTE>
> The DeleteFile function fails if an application attempts to delete a file that is open for normal I/O or as a memory-mapped file.
>
> The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed. Subsequent calls to CreateFile to open the file fail with ERROR_ACCESS_DENIED.
>
> </QUOTE>

The first statement is true under the DOS-based Windows systems -
DeleteFile fails with ERROR_ACCESS_DENIED when that file has one or more
open file handles.

The second statement is true under the NT Windows systems - DeleteFile
will fail with ERROR_SHARING_VIOLATION if at least one handle to it
isn't opened with FILE_SHARE_DELETE, and ERROR_ACCESS_DENIED if it has a
memory mapping.

Results from the attached test under Windows 7 x64:

Testing with no sharing, same process and no file mapping
DeleteFile: Failed : 00000020
Done

Testing with FILE_SHARE_DELETE, same process and no file mapping
DeleteFile: OK
Done

Testing with no sharing, separate process and no file mapping
DeleteFile: Failed : 00000020
Done

Testing with FILE_SHARE_DELETE, separate process and no file mapping
DeleteFile: OK
Done

Testing with no sharing, same process and file mapping
DeleteFile: Failed : 00000020
Done

Testing with FILE_SHARE_DELETE, same process and file mapping
DeleteFile: Failed : 00000005
Done

Testing with no sharing, separate process and file mapping
DeleteFile: Failed : 00000020
Done

Testing with FILE_SHARE_DELETE, separate process and file mapping
DeleteFile: Failed : 00000005
Done

Results from the attached test under Windows 98 SE:

Testing with no sharing, same process and no file mapping
DeleteFile: Failed : 00000005
Done

Testing with FILE_SHARE_DELETE, same process and no file mapping
CreateFile failed : 00000057
DeleteFile: OK
Done

Testing with no sharing, separate process and no file mapping
DeleteFile: Failed : 00000005
Done

Testing with FILE_SHARE_DELETE, separate process and no file mapping
CreateFile failed : 00000057
DeleteFile: OK
Done

Testing with no sharing, same process and file mapping
DeleteFile: Failed : 00000005
Done

Testing with FILE_SHARE_DELETE, same process and file mapping
CreateFile failed : 00000057
DeleteFile: OK
Done

Testing with no sharing, separate process and file mapping
DeleteFile: Failed : 00000005
Done

Testing with FILE_SHARE_DELETE, separate process and file mapping
CreateFile failed : 00000057
DeleteFile: OK
Done