Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try fix WriteableBitmap hangs when source bitmap is rendered on other thread #4425

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lindexi
Copy link
Member

@lindexi lindexi commented Apr 18, 2021

Fixes Issue #4396

Description

See #4396

Why

Why the background task will dead lock? Because the background task will get the lock in WriteableBitmap.InitFromBitmapSource method.

    public sealed class WriteableBitmap : BitmapSource
    {
        private void InitFromBitmapSource(
            BitmapSource source
            )
        {
        	// Ignore code
        	 _syncObject = source.SyncObject;
            lock (_syncObject)
            {
            	// Ignore code
            }
        }
    }

And the source is a TransformedBitmap which was created in ProcessImageAsync method and running in background task.

// The Demo code
		private void ProcessImageAsync(string filePath)
		{
			TransformedBitmap tb = new TransformedBitmap(new BitmapImage(new Uri(filePath)), new RotateTransform(90));

			CopyBitmapSourceToUi(tb);

			_ = new WriteableBitmap(tb);
		}

But the source.SyncObject will be used in main thread when Render.

 	PresentationCore.dll!System.Windows.Media.Imaging.BitmapSource.UpdateBitmapSourceResource(System.Windows.Media.Composition.DUCE.Channel channel = {System.Windows.Media.Composition.DUCE.Channel}, bool skipOnChannelCheck)
 	PresentationCore.dll!System.Windows.Media.Imaging.BitmapSource.UpdateResource(System.Windows.Media.Composition.DUCE.Channel channel, bool skipOnChannelCheck)
 	PresentationCore.dll!System.Windows.Media.Imaging.BitmapSource.AddRefOnChannelCore(System.Windows.Media.Composition.DUCE.Channel channel = {System.Windows.Media.Composition.DUCE.Channel})
 	PresentationCore.dll!System.Windows.Media.Imaging.BitmapSource.System.Windows.Media.Composition.DUCE.IResource.AddRefOnChannel(System.Windows.Media.Composition.DUCE.Channel channel)
 	PresentationCore.dll!System.Windows.Media.RenderData.System.Windows.Media.Composition.DUCE.IResource.AddRefOnChannel(System.Windows.Media.Composition.DUCE.Channel channel = {System.Windows.Media.Composition.DUCE.Channel})
 	PresentationCore.dll!System.Windows.UIElement.RenderContent(System.Windows.Media.RenderContext ctx, bool isOnChannel)
 	PresentationCore.dll!System.Windows.Media.Visual.UpdateContent(System.Windows.Media.RenderContext ctx = {System.Windows.Media.RenderContext}, System.Windows.Media.VisualProxyFlags flags, bool isOnChannel)
 	PresentationCore.dll!System.Windows.Media.Visual.RenderRecursive(System.Windows.Media.RenderContext ctx = {System.Windows.Media.RenderContext})

The main thread will use the same SyncObject in BitmapSource.UpdateBitmapSourceResource

     public abstract class BitmapSource : ImageSource, DUCE.IResource
     {
        internal virtual void UpdateBitmapSourceResource(DUCE.Channel channel, bool skipOnChannelCheck)
        {
                // Ignore code
                // We may end up loading in the bitmap bits so it's necessary to take the sync lock here.
                lock (_syncObject)
                {
                    channel.SendCommandBitmapSource(
                        _duceResource.GetHandle(channel),
                        DUCECompatiblePtr
                        );
                }
            }
        }
     }

The main thread will waitting the _syncObject which be used in background task in WriteableBitmap.InitFromBitmapSource method.

But the background task now waitting the main thread in MediaSystem.Startup. So the main thread wait background task to release the _syncObject lock and the background task wait main thread.

How

By splitting the original Unlock method, we are able to enter the InitFromBitmapSource method when creating a WriteableBitmap object in the background thread without having to wait for the main thread to complete and release the lock. This allows the _syncObject lock to be released as quickly as possible when creating a WriteableBitmap object in the background thread. Since the lock is released before the background thread enters waiting for the main thread, the main thread can safely wait for the lock. The main thread will not wait for each other with the background thread.

Customer Impact

Regression

Testing

Demo test: https://github.com/lindexi/lindexi_gd/tree/82c1dc09816d7a15214a167cae78f215a3393d6c/BitmapSourceTest

Risk

@lindexi lindexi requested a review from a team as a code owner April 18, 2021 08:49
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Apr 18, 2021
@ghost ghost requested review from fabiant3, ryalanms and SamBent April 18, 2021 08:49
@lindexi
Copy link
Member Author

lindexi commented Jul 19, 2021

@ryalanms @fabiant3 Any news?

@dipeshmsft dipeshmsft self-assigned this Dec 23, 2021
@pchaurasia14 pchaurasia14 added the Community Contribution A label for all community Contributions label Jul 20, 2022
@ghost ghost assigned lindexi Jul 20, 2022
@miloush
Copy link
Contributor

miloush commented Aug 2, 2023

@lindexi can you please be more descriptive in the PR, what is the bug and how does your PR fix it?

@lindexi
Copy link
Member Author

lindexi commented Aug 3, 2023

@miloush Yes, I will add

@Kuldeep-MS
Copy link
Member

@lindexi - Could you please rebase this PR and resolve the build error? I attempted to do it myself, but I don't have the necessary access.

@lindexi lindexi force-pushed the t/lindexi/WriteableBitmapHangMain branch from 2f3bbfe to 15b303f Compare February 7, 2025 01:39
@lindexi
Copy link
Member Author

lindexi commented Feb 7, 2025

@Kuldeep-MS Thank you and I rebase the PR.

@h3xds1nz
Copy link
Member

h3xds1nz commented Feb 7, 2025

@lindexi The build fails due to "Void" instead of "void" in method signatures:

  • UnlockWithoutSubscribeToCommittingBatch
  • SubscribeToCommittingBatchAndWritePostscript

I also don't like the var, that should fail by default 😀

@lindexi lindexi force-pushed the t/lindexi/WriteableBitmapHangMain branch from 15b303f to b8edb20 Compare February 7, 2025 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants