search

sharpdx / SharpDX

SharpDX GitHub Repository
http://sharpdx.org
MIT License
1.69k stars 641 forks source link

[DWrite] Crash caused by bad ref count on FontFile created in FontCollectionLoader implementation. #1009

Open Jayonas opened 6 years ago

Jayonas commented 6 years ago

I had a periodic crash occuring during cleanup/finalization in various different tests, and the root cause turned out to be an incorrect ref count on an IDWriteFontFile. The ref count was short by one, so the IDWriteFontFile had already been deleted by the time the finalizer came around and freed the managed FontFile, which in turn tried to delete the IDWriteFontFile again (because I have Configuration.EnableReleaseOnFinalizer enabled). This usually resulted in an access violation.

I create FontFile instances in my implementation of a FontCollectionLoader. While troubleshooting I moved the FontFile to a broader scope (outside the loader's callback) so that I could watch its ref count, and it goes to zero sometime between when my FontFileEnumerator returns it to DWrite and when I receive the fully assembled FontCollection back from DWrite. It seems reasonable for DWrite to no longer have any references to it at that point, but one ref should still remain since I hadn't disposed the managed object yet. Anytime after I get my FontCollection back, using the NativePointer on that FontFile instance caused a crash. I didn't originally see the crash until cleanup/finalization because under normal circumstances that's the next time that the FontFile.NativePointer was used.

My workaround is to artificially increment the ref count on the FontFile using Marshal.AddRef right after I create it. I double-checked that this doesn't cause a memory leak by profiling an app that loads and renders the fonts hundreds of times in a loop and I didn't see any FontFile leaks.

Another thing that fixes the crash is calling GC.SuppressFinalize on the FontFile but that seems more hacky because it's more addressing a symptom rather than the root problem.

I'm not entirely sure whether this is a problem with all FontFile usages or if it's specific to using FontFile inside of a FontCollectionLoader callback. There is certainly a bunch of marshaling (including FontFile instances) that occurs when implementing a FontCollectionLoader and corresponding FontFileEnumerator in managed code, so it's plausible that something is going wrong with that specifically. I'd also like to hope that this kind of ref count bug would have already been found and fixed if it affected any managed instance of FontFile anywhere.

(Note that I'm basing this off my usage of SharpDX 3.1.1, which I realize is out of date, but it also doesn't look like much has happened in the DWrite code since then so I think it's likely that this is still an issue.)

Jayonas commented 5 years ago

I've recently updated SharpDX, and I can confirm that this issue still exists in 4.2.0.