SkiaSharp: [BUG] SkiaSharp can call WM_PAINT handler and random COM callbacks when creating or obtaning objects

Hi Andrew, thanks for your insight on this - that’s really valuable info. Message hooks to filter paint messages sounds like too much fun.

This is all a bit of a mess, but for understood reasons. I think we agree SkiaSharp could improve things with a non-pumping lock, or at least a way to plugin your own sync mechanism.

Unless, or until this can be fixed SkiaSharp I’m tempted to just disable the pumping wait entirely. I feel like I’m more exposed by that than anything else the app or any plugins might do. But I’m sure that’ll bite me somewhere else 😃

@mattleibow you were right! TIL, plain c# locks (ie., Monitor.Enter/Monitor.Exit) also pump on an STA thread 👀 https://gist.github.com/noseratio/0d93133b826339c7eaf49c9feec142e3#file-winformsapp-cs-L60

Now I wonder if there’s any managed blocking synchronization primitive that doesn’t pump in this case, besides Thread.Sleep…

Edited: and here’s where the pumping magic is happening: https://github.com/dotnet/runtime/blob/0c0bd5a4066bc9b1a165a45e3553dfa14b87729b/src/coreclr/vm/threads.cpp#L3342

@noseratio I think using lock also had this issue: https://gist.github.com/retran/b57e4db1a173048c2cee49ac6d523fc2

@mattleibow I might be wrong but I think in this particular case it’s caused by Thread.CurrentThread.Join(100), where they call it on the main UI thread. This call does indeed pump:

                try
                {
                    Debug.Assert(Thread.CurrentThread.ManagedThreadId == mainThreadId);
                    throwOnPaint = true;
                    Thread.CurrentThread.Join(100);
                    Lock();
                }

I think, Monitor.Enter/Monitor.Exit should be mapping to Win32 EnterCriticalSection/LeaveCriticalSection directly, maybe with some added “spin-before-sleep” improvement, but I can’t find anything confirming that in the dotnet repo right away. I might try modifying the code you linked to verify this theory 😃

I think that Monitor.Enter/Monitor.Exit (which is used by C# lock { … }) should be fine, although I haven’t verified it.

@noseratio I think using lock also had this issue: https://gist.github.com/retran/b57e4db1a173048c2cee49ac6d523fc2