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.

Sign up for GitHub

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

Jump to bottom

Dispose empty sets and try/catch exceptions #10955

Merged
merged 1 commit into from
Oct 27, 2022
Merged

Dispose empty sets and try/catch exceptions #10955

merged 1 commit into from
Oct 27, 2022

Conversation

mattleibow
Copy link
Member

Description of Change

There seems to be a bug in the iOS/Mac Catalyst SDK where empty "collections" that are generic all use the same pointer. This causes the first reference to win and the rest throw.

This PR has a very strong error-situation-avoidance feel to it, see comments...

Issues Fixed

Workarounds for:

mattleibow
mattleibow commented Oct 27, 2022
View reviewed changes
src/Essentials/src/Platform/WindowStateManager.ios.cs
return windowScene?.Windows.FirstOrDefault();
try
{
using var scenes = UIApplication.SharedApplication.ConnectedScenes;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First level of attack, make sure to dispose the "other" set so that there is no managed object around to get a miss-type.

src/Essentials/src/Platform/WindowStateManager.ios.cs
Comment on lines +110 to +115
catch (InvalidCastException)
{
// HACK: Workaround for https://github.com/xamarin/xamarin-macios/issues/13704
// This only throws if the collection is empty.
return null;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second level, if we do get an exception, we know it is because the set was empty, so do empty things.

src/Core/src/Platform/iOS/ApplicationExtensions.cs
{
foreach (var u in connectionOptions.UserActivities)
using var activities = connectionOptions.UserActivities;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First-point-five level, make sure we also dispose the error set because there may be another set later on.

src/Core/src/Platform/iOS/ApplicationExtensions.cs
Comment on lines +73 to +77
catch (InvalidCastException)
{
// HACK: Workaround for https://github.com/xamarin/xamarin-macios/issues/13704
// This only throws if the collection is empty.
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second-point-five protection, if we do somehow throw, then we know there is no user activity.

@rmarinho rmarinho merged commit dbfaf45 into main Oct 27, 2022
@rmarinho rmarinho deleted the dev/ios-fix branch October 27, 2022 14:43
rmarinho pushed a commit that referenced this pull request Oct 27, 2022
@mattleibow @rmarinho
Dispose empty sets and try/catch exceptions (#10955)
20a28a4 
Workaround for xamarin/xamarin-macios#13704
rmarinho pushed a commit that referenced this pull request Oct 27, 2022
@mattleibow @rmarinho
Dispose empty sets and try/catch exceptions (#10955)
10a8bba 
Workaround for xamarin/xamarin-macios#13704
rmarinho pushed a commit that referenced this pull request Oct 27, 2022
@mattleibow @rmarinho
Dispose empty sets and try/catch exceptions (#10955)
c94e11e 
Workaround for xamarin/xamarin-macios#13704
rmarinho pushed a commit that referenced this pull request Oct 27, 2022
@mattleibow @rmarinho
Dispose empty sets and try/catch exceptions (#10955)
4a0227e 
Workaround for xamarin/xamarin-macios#13704
Redth added a commit that referenced this pull request Oct 27, 2022
@rmarinho @dotnet-maestro
@mattleibow @Redth
Cherrypick fron net7 to release branch (#10957)
a99302b 
* Squashed commit of the following: (#10942)

commit 22b3033
Merge: 50866a7 95ee1a3
Author: Matthew Leibowitz <mattleibow@live.com>
Date:   Thu Oct 27 11:10:47 2022 +0200

    Merge branch 'net7.0' into darc-net7.0-45fa6a48-68f2-4992-95f2-e40e8e14afd5

commit 50866a7
Author: Jonathan Dick <jondick@gmail.com>
Date:   Wed Oct 26 21:22:02 2022 -0400

    Bump net6 version for SR7

commit 8edadd3
Author: Rui Marinho <me@ruimarinho.net>
Date:   Wed Oct 26 18:55:17 2022 +0100

    Update NuGet.config

commit b7c5411
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 17:48:49 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.24

    Microsoft.tvOS.Sdk
     From Version 16.0.1475 -> To Version 16.0.1477

commit 4c57036
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 17:48:09 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.24

    Microsoft.iOS.Sdk
     From Version 16.0.1475 -> To Version 16.0.1477

commit 89f722e
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 16:51:31 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.21

    Microsoft.tvOS.Sdk
     From Version 16.0.1475 -> To Version 16.0.1476

commit 9dc4a41
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 16:51:01 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.21

    Microsoft.iOS.Sdk
     From Version 16.0.1475 -> To Version 16.0.1476

commit dde2135
Author: Rui Marinho <me@ruimarinho.net>
Date:   Wed Oct 26 17:46:35 2022 +0100

    Add Nuget.config feed

commit bad3509
Author: GitHub Actions Autoformatter <autoformat@example.com>
Date:   Wed Oct 26 16:41:26 2022 +0000

    Auto-format source code

commit 6d56d6c
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 16:40:27 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.22

    Microsoft.MacCatalyst.Sdk
     From Version 15.4.2370 -> To Version 15.4.2371

commit 2da2d4d
Author: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Date:   Wed Oct 26 16:40:10 2022 +0000

    Update dependencies from https://github.com/xamarin/xamarin-macios build 20221026.22

    Microsoft.macOS.Sdk
     From Version 12.3.2370 -> To Version 12.3.2371

Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
# Conflicts:
#	eng/Version.Details.xml
#	eng/Versions.props

* Dispose empty sets and try/catch exceptions (#10955)

Workaround for xamarin/xamarin-macios#13704

* Update MAUI's net6 version

Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Jonathan Dick <jondick@gmail.com>
@samhouts samhouts added area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info platform/iOS 🍎 platform/windows 🪟 labels Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2023
@samhouts samhouts added fixed-in-6.0.547 Look for this fix in 6.0.547 SR7! fixed-in-6.0.548 fixed-in-7.0.52 Look for this fix in 7.0.52 SR1.1! fixed-in-7.0.49 Look for this fix in 7.0.49 GA! labels Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rolfbjarne rolfbjarne rolfbjarne approved these changes

@Redth Redth Redth approved these changes

Assignees
No one assigned
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-6.0.547 Look for this fix in 6.0.547 SR7! fixed-in-6.0.548 fixed-in-7.0.49 Look for this fix in 7.0.49 GA! fixed-in-7.0.52 Look for this fix in 7.0.52 SR1.1! platform/iOS 🍎 platform/windows 🪟
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mattleibow @rolfbjarne @Redth @rmarinho @samhouts