[Bug] Using fonts from a non-UI thread results in exception with HRESULT 0x8001010E (RPC_R_WRONG_THREAD) on UWP

johnshardman commented 4 years ago

Description

Using fonts from a non-UI thread, even when the font is not being used in a page's UI hierarchy, results in an exception with HRESULT 0x8001010E (RPC_R_WRONG_THREAD) on UWP.

This prevents pages being prepared away from the UI thread before they are actually required, meaning that an obvious performance improvement technique cannot be used on UWP if fonts are used at all (the same problem happens if XAML is used as well).

Steps to Reproduce

Add a Button to a page, with the Button defined as follows:

                new Button
                {
                    Text = "Use a font on a non-UI thread",
                    Command = new Command(() =>
                    {
                        Task.Run(() =>
                        {
                            Xamarin.Forms.Label label = new Xamarin.Forms.Label
                            {
                                Text = "Hello",
                                FontAttributes = FontAttributes.Italic
                            };
                        });
                    })
                }

Run the code and press the Button
The exception occurs, even though the View with the FontAttribute being set is not hooked into the UI hierarchy.

Expected Behavior

No exception. Font-related properties should be set as per the code.

Actual Behavior

Exception with HRESULT 0x8001010E (RPC_R_WRONG_THREAD)

Basic Information

Version with issue: 4.3.0.991211 (but this has been a problem for a long time)
IDE: VS2019
Platform Target Frameworks:
- UWP: 18362

kingces95 commented 4 years ago

Not sure we're gonna support this. True that it runs slower but that's just how it goes. Is there some other precedent for allowing this on UWP regardless of XF?

johnshardman commented 4 years ago

Not sure we're gonna support this. True that it runs slower but that's just how it goes. Is there some other precedent for allowing this on UWP regardless of XF?

The issue is that a technique (recommended by Xamarin University staffers when that was still running) used for XF apps on Android and iOS cannot currently be used on UWP, meaning that apps either have to not use the technique at all, or have to have different code paths (and hence more testing effort) for UWP to Android & iOS (which is what XF is supposed to minimise). I do not know whether a non-XF UWP app can manipulate fonts on a non-UI thread - hopefully somebody else can answer that.

samhouts commented 4 years ago

Issue8840.zip

   at Windows.UI.Xaml.Application.get_Resources()
   at Xamarin.Forms.Platform.UWP.FontExtensions.GetFontSize(NamedSize size)
   at Xamarin.Forms.Platform.UWP.WindowsBasePlatformServices.GetNamedSize(NamedSize size, Type targetElementType, Boolean useOldSizes)
   at Xamarin.Forms.Label.Xamarin.Forms.Internals.IFontElement.FontSizeDefaultValueCreator()
   at Xamarin.Forms.FontElement.FontSizeDefaultValueCreator(BindableObject bindable)
   at Xamarin.Forms.BindableObject.CreateAndAddContext(BindableProperty property)
   at Xamarin.Forms.BindableObject.GetValue(BindableProperty property)
   at Xamarin.Forms.FontElement.OnFontAttributesChanged(BindableObject bindable, Object oldValue, Object newValue)
   at Xamarin.Forms.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent)
   at Xamarin.Forms.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes)
   at Xamarin.Forms.BindableObject.SetValue(BindableProperty property, Object value, Boolean fromStyle, Boolean checkAccess)
   at Xamarin.Forms.Label.set_FontAttributes(FontAttributes value)
   at Issue8840.MainPage.<>c.<.ctor>b__0_1() in C:\Users\saman\Downloads\Issue8840\Issue8840\Issue8840\MainPage.xaml.cs:line 27
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__276_1(Object obj)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)

charlesroddie commented 3 years ago

We are getting crashes on user systems because of this.

This involves GetFontSize(NamedSize size) when rendering a Label.

I have no idea why this method using NamedSIze is being used: we are specifying FontSize explicitly.

charlesroddie commented 3 years ago

Looking up DefaultFontSizeValueCreator in this repo gives some very strange things:

https://github.com/xamarin/Xamarin.Forms/search?q=FontSizeDefaultValueCreator

It's being defined 11 times on different views, not just once, so there is an abstraction failure.
It's never explicitly used. Maybe it's called with some reflection. I hope this architectural flaw goes away with MAUI.
The implementation references Device.GetNamedSize which is really inappropriate as the Forms object then depends on the platform projects for how it is constructed. The differences where possible should be in rendering not in the View construction.

Since I can't identify where Device.GetNamedSize is called, I can't say why it's called unexpectedly. One of the many XF bugs which is either caused by, or whose resolution is made harder by, not using .Net properly.

charlesroddie commented 3 years ago

The following code crashes on UWP:

Label(FontSize = 10.)
Label(Font = Font.OfSize("FontFamily", 10.))
Label(Font = Font.OfSize("FontFamily", NamedSize.Small))

The problem comes here:

double IFontElement.FontSizeDefaultValueCreator() =>
    Device.GetNamedSize(NamedSize.Default, (Label)this);

I lost the trail at this point because some reflection system kicks in here so don't know where FontSizeDefaultValueCreator is called. Somehow it's getting called even when the Font is specified. "Go to definition" on GetNamedSize also doesn't return any leads for the same reason.

There are so many things that need changing in this code but I think the way to get XF UWP working is to return 14.667 for NamedSize.Default .

internal static double GetFontSize(this NamedSize size)
{
  // TODO: Hmm, maybe we need to revisit this, since we no longer support Windows Phone OR WinRT.
  // These are values pulled from the mapped sizes on Windows Phone, WinRT has no equivalent sizes, only intents.
  switch (size)
  {
  case NamedSize.Default:
      if(DefaultFontSize == double.NegativeInfinity)
      {
          DefaultFontSize = (double)WApplication.Current.Resources["ControlContentThemeFontSize"];
      }
      return DefaultFontSize;
  case NamedSize.Micro:
      return 15.667;
  case NamedSize.Small:
      return 18.667;
  case NamedSize.Medium:
      return 22.667;
  case NamedSize.Large:
      return 32;
  case NamedSize.Body:
      return 14;
  case NamedSize.Caption:
      return 12;
  case NamedSize.Header:
      return 46;
  case NamedSize.Subtitle:
      return 20;
  case NamedSize.Title:
      return 24;
  default:
      throw new ArgumentOutOfRangeException("size");
  }
}

Summary

There are errors in the binding framework that cause the creation of spurious default objects (in this creating a fontsize default value even when fontsize is specified).
FontSize.Default then goes on to query UWP methods even before the rendering stage.

It's very simple to do a fix to 2, which should prevent serious crashes, with minimal impact on behaviour. (A user would have to set ControlContentThemeFontSize and then deliberately not set font sizes in order to see an effect.)

charlesroddie commented 3 years ago

A workaround is to call this before startup (from the UI thread):

Device.GetNamedSize(NamedSize.Default, typeof(Label))

xamarin / Xamarin.Forms