ScreenGrab opens the file in a shared writing mode

[CookiePLMonster]

While SaveDDSTextureToFile opens the file for writing in an exclusive mode and fails with a sharing violation if the access is denied, SaveWICTextureToFile opens files in a shared writing mode and enables multiple writers to race to write multiple screenshots to the same file.

In my project, I prototyped moving the file write to a separate thread, and only handed off the D3D surface from the main thread to the temporary writer thread. My screenshots are timestamped with a granularity of one second, so spamming enough screenshots could result in multiple attempts to write to the same file. This resulted in the following consequences:

1. Multiple threads opened the same file for writing, and none of them failed.
2. They produced a corrupted image.

Writing WIC textures utilizes IWICStream::InitializeFromFilename to create a stream. The following is stated in the Remarks:

The IWICStream interface methods do not enable you to provide a file sharing option.
To create a shared file stream for an image, use the SHCreateStreamOnFileEx function.

This wording would imply that the default file sharing option is exclusive, and the user should use the Shell function to create a shared stream. However, the opposite is true, at least on Windows 10 22H2:

1. Opening the same filename for shared writing prior to using InitializeFromFilename lets it succeed with no issues.
2. Opening the same filename for exclusive writing prior to using InitializeFromFilename blocks execution inside of that function, probably waiting for the file to become available.

---

This raises multiple questions:

1. Should ScreenGrab stop using IWICStream and instead use SHCreateStreamOnFileEx to create an exclusive stream? The following replacement works fine and returns a sharing violation if concurrent writes are attempted:

    ComPtr<IStream> stream;
    hr = SHCreateStreamOnFileEx(fileName, STGM_WRITE|STGM_SHARE_EXCLUSIVE|STGM_CREATE,
    	FILE_ATTRIBUTE_NORMAL, TRUE, nullptr, stream.GetAddressOf());
    if (FAILED(hr))
    	return hr;
    
    auto_delete_file_wic delonfail(stream, fileName);

   However, it also creates a dependency on Shlwapi.lib, which the current solution does not have.
2. Should the docs of IWICStream::InitializeFromFilename be updated to say that the default file share mode is shared, and SHCreateStreamOnFileEx should be used for exclusive access, essentially flipping the current remark? Or perhaps the WIC stream is opened exclusively when reading, and not writing? EDIT: I checked locally, opening two WIC streams for the same file with GENERIC_READ succeeds. However, opening the first stream with GENERIC_READ and the second with GENERIC_WRITE fails with a sharing violation...

[walbourn]

I purposely avoided the shell dependency since it's not supported on Xbox platforms. There's also been some feedback that for performance reasons I should avoid CreateStreamOnHGlobal.

I generally have made all my reads sharable, but I expected writes to be exclusive.

[CookiePLMonster]

I generally have made all my reads sharable, but I expected writes to be exclusive.

I agree and that's what I expected when I willingly made the writes "race" to potentially opening the same file, but apparently WIC Streams are more decoding-oriented than encoding-oriented, as evidenced by the Remarks in their pages only mentioning decoding and never encoding.

However, is there a reasonable way ScreenGrab could ensure exclusivity without introducing dependencies? Or should this be handled by the user/application (for example with a binary semaphore)? If ScreenGrab cannot reasonably account for this, then perhaps this discrepancy between the DDS and WIC writers should be documented.

It would be a trivial problem if there was a way to open an output IStream from a file HANDLE, as then CreateFile2 could be used just like in the DDS writer - but I haven't found a way to do it.

[walbourn]

I guess I don't understand why you'd be creating the same filename with the DDS writer and the WIC writer? Are you not making use of file extentions?

[CookiePLMonster]

I'm not, I only mentioned DDS vs WIC in terms of behaviour parity. In my case, I was creating the same filenames with multiple threads using the WIC writer because my screenshots had filenames with YYYY-MM-DD-HH-MM-SS in the name, and thus spamming the screenshot key resulted in multiple attempts to write the same file. The design I prototyped here was shaky that's for sure (and I have dropped the idea of threading the writes since), but it would have worked correctly if the files were opened exclusively (like the DDS writer does).

[walbourn]

Ah, thanks for the clarification.