WICTextureLoader is using "leaky singleton" for WIC factory

[jardik]

WICTextureLoader is using "leaky singleton" for WIC factory. As such, it cannot be used in code, where no-leaking is a requirement. I wanted to use it in preview handler Windows Explorer plugin, but I cannot - plugin must not contain leaks, as it can be loaded and unloaded dynamicaly and I am not in control of this loading/unloading. Since C++11, static initialization must be thread safe. Code of the library requires C++17 and current implementation using InitOnceExecuteOnce should not be needed. Standard function-local static variable can be used to perform the initialization and when library is unloaded, dtor would be called to release the factory. This is, however, also not ideal, as it can be too late, if COM gets uninitialized before the library is actually unloaded. Probably best would be if there was no global variable at all - making WICTextureLoader a class with factory as member variable and all the function as member functions. You probably won't be making this change, but you should probably at least document this "leak", so someone doesn't use the library by mistake in code, where it shouldn't be used (any unloadable library).

Edit:
I believe that current implementation also suffers from threading issues. Shouldn't the factory only be used from the thread that created it?

[walbourn]

My understanding is that IWICImagingFactory methods are thread-safe. I'll double-check though.

I don't rely on 'magic statics' for Windows because it's built in some contexts for toolsets that don't have it enabled. For non-Win32 platforms I rely on C++11 magic statics.

For DirectXTex, I provided a SetWICFactory which can be used with nullptr to release things. The app has to be careful about when it does this, but it allows the 'leak' to be fixed. I could do something similar with DirectXTK.

The class idea is interesting, but it would be a breaking change for existing clients as well as require I pull in ScreenGrab's WIC writer too...

[jardik]

I don't rely on 'magic statics' for Windows because it's built in some contexts for toolsets that don't have it enabled.

If you introduce non-standard behavior using a compiler switches, than you just can't relly on anything in the standard or the standard library, basicly breaking your application. Something in the standard library could use a static variable and you just broke an assumption that it will be thread-safe. If application disables thread safe static initializers, then it probably knows it doesn't use threads and as such, there is no need to be concerned whether or not the static will be thread safe, allowing you to use it.

Anyway, I just wanted to point out that this can be an issue, at least for some uses, and it may be worth to document it at least. I know many developers just find a library that they think they could use to save their time, without doing full review of its code, and they could end up with "broken" applications / libraries.

My understanding is that IWICImagingFactory methods are thread-safe. I'll double-check though.

I just did some digging in registry (HKEY_CLASSES_ROOT\CLSID{317D06E8-5F24-433D-BDF7-79CE68D8ABC2}\InProcServer32) and it has ThreadingModel set to Both, so it should be ok?

[walbourn]

Thanks for the report. What do you think of having a GetWICFactoryForTK / SetWICFactoryForTK added to the DirectX Tool Kit with basically the same design as in DirectXTex?

[jardik]

Thanks for the report. What do you think of having a GetWICFactoryForTK / SetWICFactoryForTK added to the DirectX Tool Kit with basically the same design as in DirectXTex?

Hello. Unfortunatelly I found out I won't be able to use this library, even if this change was introduced. What I am doing is making a preview handler for a file type (as COM DLL) and I need to track every object to correctly implement DllCanUnloadNow.

It will be easier for me to just copy-paste texture loading code from this library and use "per-preview handler" WIC factory, which will get released together with the preview handler, and having no global state at all.

[walbourn]

You are of course free to cut & paste per the terms of the MIT license, but you could also look at using DirectXTex rather than the DirectX Tool Kit simplified modules to do the I/O as well.