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

New Blocks marshalling in 8.0 sometimes fails with ArgumentException #18161

Closed
filipnavara opened this issue Apr 27, 2023 · 3 comments · Fixed by #18277
Closed

New Blocks marshalling in 8.0 sometimes fails with ArgumentException #18161

filipnavara opened this issue Apr 27, 2023 · 3 comments · Fixed by #18277
Assignees
@rolfbjarne
Labels
bug If an issue is a bug or a pull request a bug fix regression The issue or pull request is a regression
Milestone
Future

Comments

@filipnavara
Copy link
Member 

An item with the same key has already been added.

System.ThrowHelper.ThrowArgumentException (:0)
System.Runtime.CompilerServices.ConditionalWeakTable<System.Delegate,ObjCRuntime.BlockCollector>.Add (:0)
ObjCRuntime.Runtime.ReleaseBlockWhenDelegateIsCollected (:0)
ObjCRuntime.Runtime.CreateBlockProxy (:0)
ObjCRuntime.Runtime.CreateBlockProxy (:0)
ObjCRuntime.Runtime.create_block_proxy (:0)
ObjCRuntime.Messaging.void_objc_msgSend_NativeHandle_NativeHandle_NativeHandle (:0)
WebKit.WKNavigationDelegate_Extensions.DecidePolicy (:0)
MailClient.Mobile.iOS.Renderers.WebView.CustomWebViewNavigationDelegate.DecidePolicy (/Users/filipnavara/Projects/emclient-maui/MailClient.Mobile/MailClient.Mobile.iOS/Renderers/WebView/CustomWebViewNavigationDelegate.cs:73)
UIKit.UIApplication.xamarin_UIApplicationMain (:0)
UIKit.UIApplication.UIApplicationMain (:0)
UIKit.UIApplication.Main (:0)
MailClient.Mobile.iOS.Application.Main (/Users/filipnavara/Projects/emclient-maui/MailClient.Mobile/MailClient.Mobile.iOS/Main.cs:17)

The code in question:

	class CustomWebViewNavigationDelegate : WKNavigationDelegate
	{
		readonly CustomWebViewRenderer renderer;
		readonly IWKNavigationDelegate navigationDelegate;

		public CustomWebViewNavigationDelegate(CustomWebViewRenderer renderer, IWKNavigationDelegate navigationDelegate)
		{
			this.renderer = renderer;
			this.navigationDelegate = navigationDelegate;
		}
		
		...
		
		public override void DecidePolicy(WKWebView webView, WKNavigationAction navigationAction, Action<WKNavigationActionPolicy> decisionHandler)
		{
			if (navigationAction.TargetFrame?.MainFrame == false)
				((WebViewEx)renderer.Element).OnIOSPageFrameStarted(new PageEventArgs(navigationAction.Request.Url.AbsoluteUrl.ToString()));

			var request = new RequestModel(new Uri(navigationAction.Request.Url.AbsoluteString));
			if (((WebViewEx)renderer.Element).ShouldOverrideUrlLoading(request))
				decisionHandler(WKNavigationActionPolicy.Cancel);
			else
				navigationDelegate.DecidePolicy(webView, navigationAction, decisionHandler);  // <-- CRASHES HERE
		}

		...			
	}
@rolfbjarne rolfbjarne added bug If an issue is a bug or a pull request a bug fix regression The issue or pull request is a regression labels Apr 27, 2023
@rolfbjarne rolfbjarne added this to the Future milestone Apr 27, 2023
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue May 15, 2023
@rolfbjarne
[runtime] Add support for keeping the same block alive multiple times. …
28619f9 
…Fixes dotnet#18161.

We can be asked to keep the same block+delegate pair alive multiple times, so
add support for keeping track how many times a block must be freed once the
corresponding delegate is collected by the GC.

Fixes dotnet#18161.
@rolfbjarne rolfbjarne mentioned this issue May 15, 2023
Merged
rolfbjarne added a commit that referenced this issue May 17, 2023
@rolfbjarne
[runtime] Add support for keeping the same block alive multiple times. …
70de850 
…Fixes #18161. (#18277)

We can be asked to keep the same block+delegate pair alive multiple
times, so add support for keeping track how many times a block must be freed once
the corresponding delegate is collected by the GC.

Fixes #18161.
@rolfbjarne
Copy link
Member

/sudo backport release/7.0.2xx-xcode14.3

@rolfbjarne rolfbjarne mentioned this issue May 19, 2023
Closed
vs-mobiletools-engineering-service2 pushed a commit to vs-mobiletools-engineering-service2/xamarin-macios that referenced this issue May 19, 2023
@rolfbjarne
[runtime] Add support for keeping the same block alive multiple times. …
0b13537 
…Fixes dotnet#18161.

We can be asked to keep the same block+delegate pair alive multiple times, so
add support for keeping track how many times a block must be freed once the
corresponding delegate is collected by the GC.

Fixes dotnet#18161.
Merged
@pdonovan
Copy link

I found this ticket via googling, I have the same error in my own WKNavigationDelegate using Xcode 14.3 and Xamarin.iOS 16.4.0.6. It seems like this is a regression - what package/tool/update am I waiting on to get this fix?

@filipnavara
Copy link
Member Author

@pdonovan You can use this workaround: #18302 (comment)

dalexsoto pushed a commit that referenced this issue May 24, 2023
@vs-mobiletools-engineering-service2 @rolfbjarne
[release/7.0.2xx-xcode14.3] [runtime] Add support for keeping the sam…
868ba2e 
…e block alive multiple times. Fixes #18161. (#18303)

We can be asked to keep the same block+delegate pair alive multiple
times, so
add support for keeping track how many times a block must be freed once
the
corresponding delegate is collected by the GC.

Fixes #18161.


Backport of #18277

Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rolfbjarne rolfbjarne

Labels
bug If an issue is a bug or a pull request a bug fix regression The issue or pull request is a regression
Projects
None yet
Milestone
Future
3 participants
@rolfbjarne @pdonovan @filipnavara