Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] Popup Dismiss passing an object back crashes unless invoked on main thread iOS only #1619

Open
stephenhauck opened this issue Sep 16, 2021 · 6 comments
Labels
bug Something isn't working. Breaky break. s/unverified This issue needs verification/reproduction by a team member. PRs cannot be accepted/merged.

Comments

@stephenhauck
Copy link

Dismiss will crash with a UI View error on iOS, android seems to work fine
Dismiss(result);

Steps to Reproduce

  1. Download my example app here https://github.com/stephenhauck/ZXingPopup
  2. Change this code MainThread.BeginInvokeOnMainThread(() => { Dismiss(result); });

To Dismiss(result);

  1. Run app on iOS device

Running on latest stable everything ...

@stephenhauck stephenhauck added bug Something isn't working. Breaky break. s/unverified This issue needs verification/reproduction by a team member. PRs cannot be accepted/merged. labels Sep 16, 2021
@stephenhauck
Copy link
Author

When they cancel I pass back null and all is well ...

@SkyeHoefling
Copy link
Contributor

The title of this bug says it only works on the main UI thread for iOS. The popups are required to operate on the main UI thread, any exceptions caused because it is not operating on the main UI thread is an expected behavior. There is no trick or workaround to this as the popup operation will need to be marshaled from a background thread to the main UI thread. Blindly marshaling all threads to the main UI thread can cause issues with the thread pool which may slow the app down or cause other performance issues.

cc: @pictos

@stephenhauck
Copy link
Author

ok but it's not our job to call "invoke on main thread" right ?
That should be handled in the "Dismiss" ?
When I "invoke on main thread" a blink occurs and it's just not right

@pictos
Copy link
Contributor

pictos commented Sep 23, 2021

@ahoefling this is a tricky one... Normally frameworks don't do the work to invoke things in the UI thread. Is the responsibility of the mobile dev to make sure the UI changes are happening in the UI thread. CollectionView had an issue because of that

I would say that is not the job of the lib to force it to be on the UI thread because doing this in best way possible will be very hard.

but it's not our job to call "invoke on main thread" right?

I would say if you, as a mobile dev, are doing some work on the background thread is it your job to return to the main thread. Like

void Do() {
Task.Run( ( ) => async {
    string result = await httpClient.GetString("SomeUrl").ConfigureAwait(false);
    label.Text = result;
}

If you try to do this on XF today, probably it will fail, because you are changing the label.Text in a background thread... So at the end of the day, the framework can't do this for devs.

@SkyeHoefling
Copy link
Contributor

I agree with @pictos evaluation, I don't believe this is a bug. The popup control is built tightly around the native implementations and it is expected to used from the main UI thread. Your application code can perform processing off the main thread, but when it is time to perform UI operations you will need to go back to the main UI thread.

@stephenhauck
Copy link
Author

stephenhauck commented Sep 23, 2021

I appreciate your pointing out what frameworks should and should not do and the usage of invoking on the main thread and such but I guess I'm confused ... the call to "Dismiss" should work exactly the same regardless of platform or fail exactly the same regardless of platform .... right ?

I'm in the code behind calling "Dismiss" It works find on Android but not on iOS ...
And I arrived at the view by putting it onto the navigation stack with ShowPopup like so ...
var popup = new ScanBarcodePopupView(); popup.Dismissed += Popup_Dismissed; App.Current.MainPage.Navigation.ShowPopup(popup);

Can you look at the code and tell me where I have implemented the usage incorrectly that it works fine on Android and not on iOS ?

Dismiss(result); works fine on Android...
Dismiss(result); crashes on iOS...

MainThread.BeginInvokeOnMainThread(() => { Dismiss(result); }); This is required to make it work on both platforms ....

If I have used or implemented the popup incorrectly please correct me...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working. Breaky break. s/unverified This issue needs verification/reproduction by a team member. PRs cannot be accepted/merged.
Projects
None yet
Development

No branches or pull requests

3 participants