Bug 57898 - ObjectDisposed exception caused by race condition in ScrollViewRenderer
Summary: ObjectDisposed exception caused by race condition in ScrollViewRenderer
Status: ASSIGNED
Alias: None
Product: Forms
Classification: Xamarin
Component: Android ()
Version: 2.3.4
Hardware: PC Windows
: --- normal
Target Milestone: ---
Assignee: Jimmy [MSFT]
URL:
Depends on:
Blocks:
 
Reported: 2017-07-01 10:37 UTC by giedrius
Modified: 2018-04-20 09:17 UTC (History)
7 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report for Bug 57898 on Developer Community or GitHub if you have new information to add and do not yet see a matching new report.

If the latest results still closely match this report, you can use the original description:

  • Export the original title and description: Developer Community HTML or GitHub Markdown
  • Copy the title and description into the new report. Adjust them to be up-to-date if needed.
  • Add your new information.

In special cases on GitHub you might also want the comments: GitHub Markdown with public comments

Related Links:
Status:
ASSIGNED

Description giedrius 2017-07-01 10:37:05 UTC
The crash can be reproduced by calling ScrollToAsync, and leaving the page hosting the ScrollView. It disposes of the SrollViewRenderer, however, the ScrollViewRenderer has 'OnScrollToRequested' handler which checks !_isAttached only at the beginning function, and has code to delay execution asynchronously after up to 10ms, that means it may attempt to continue to execute after the object is disposed of, causing the crash to happen.

The crash itself happens in calling of IsLayoutRequested property getter. Callstack:

Xamarin caused by: android.runtime.JavaProxyThrowable: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Xamarin.Forms.Platform.Android.ScrollViewRenderer'.
Java.Interop.JniPeerMembers.AssertSelf(IJavaPeerable self)<3a653d21f22f4786bf9d5acf69f4a22e>:0
Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeVirtualBooleanMethod(string encodedMember, IJavaPeerable self, JniArgumentValue* parameters)<3a653d21f22f4786bf9d5acf69f4a22e>:0
Android.Views.View.get_IsLayoutRequested()<cba5498252974515814d3da6e57d094c>:0
Xamarin.Forms.Platform.Android.ScrollViewRenderer.<OnScrollToRequested>d__44.MoveNext()<15c9790557774c809371f43133cc42c4>:0
Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0()<cba5498252974515814d3da6e57d094c>:0
Java.Lang.Thread.RunnableImplementor.Run()<cba5498252974515814d3da6e57d094c>:0
Java.Lang.IRunnableInvoker.n_Run(IntPtr jnienv, IntPtr native__this)<cba5498252974515814d3da6e57d094c>:0
at (wrapper dynamic-method) System.Object:ba3ec2cc-2626-44c0-ab76-80f16217bdbf (intptr,intptr)
mono.java.lang.RunnableImplementor.n_run(Native Method)
mono.java.lang.RunnableImplementor.run()RunnableImplementor.java:30
android.os.Handler.handleCallback()Handler.java:739
android.os.Handler.dispatchMessage()Handler.java:95
android.os.Looper.loop()Looper.java:158
android.app.ActivityThread.main()ActivityThread.java:7229
java.lang.reflect.Method.invoke(Native Method)
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run()ZygoteInit.java:1230
com.android.internal.os.ZygoteInit.main()ZygoteInit.java:1120

The original code where it's crashing:

async void OnScrollToRequested(object sender, ScrollToRequestedEventArgs e)
{
	if (!_isAttached)
	{
		return;
	}

	// 99.99% of the time simply queuing to the end of the execution queue should handle this case.
	// However it is possible to end a layout cycle and STILL be layout requested. We want to
	// back off until all are done, even if they trigger layout storms over and over. So we back off
	// for 10ms tops then move on.
	var cycle = 0;
	while (IsLayoutRequested)
	{
		await Task.Delay(TimeSpan.FromMilliseconds(1));
		cycle++;

		if (cycle >= 10)
			break;
	}
...

I believe this bug could be resolved by simply adding "if (!_isAttached) return;" after the 'await Task.Delay' line, it would recheck its exit condition after having asynchronously delayed the execution, so it doesn't continue execution after getting disposed and accessing IsLayoutRequested in invalid state.

There's a report in the forums too: https://forums.xamarin.com/discussion/96408/system-objectdisposedexception-cannot-access-a-disposed-object-when-using-scrollview
Comment 1 giedrius 2017-07-03 08:10:39 UTC
I have found a workaround for this crash, the idea is to delay going to the previous page, if we have requested scrolling and didn't yet receive the result.

The delaying of going back can be done by overriding 'OnBackButtonPressed', and intercepting the 'back' arrow click in the top bar in Android specific way, forwarding that event to a common handler in the page. (I haven't tackled the iOS version of my app yet, I saw on the net that there might be difficulties capturing such event on iOS). The event handler should return true to indicate that the event was handled, and default handling shouldn't be done, disallow making any more software scrolling requests and spin up a timer which checks whether there's any pending scrolling requests, if not - pop the page off the stack.

This way the crash no longer happens, as the renderer gets disposed only while there's no pending OnScrollToRequested continuation.

Some code to illustrate the fix:

// This is the only API used by my Page to initiate scrolling
private async Task scrollToBottom()
{
	if (IsDisappering || IsScrollingAutomatically)
		return;

	IsScrollingAutomatically = true;
	await View.ScrollToAsync(LastLine, ScrollToPosition.End, false);
	IsScrollingAutomatically = false;
}

protected override void OnAppearing()
{
	IsDisappering = false;
	base.OnAppearing();
}

public bool HandleBackButtonPressed()
{
	if (IsDisappering)
		return true;

	IsDisappering = true;

	// The key part is the Timer - it allows the scrolling to finish, as all of this is running in the UI thread.
	Device.StartTimer(new TimeSpan(150000), () =>
		{
			if (IsScrollingAutomatically)
				return true; // Reschedule the timer
			Navigation.PopAsync();
			return false; // Stop the timer
		}
	);

}
Comment 2 Jimmy [MSFT] 2017-07-06 16:02:47 UTC
How often are you hitting this condition? I created a sample project to try reproducing this but the ScrollView always immediately completed the scrolling so I could not pop the page quick enough to get the exception.

Do you know if this started occurring more frequently after a Xamarin.Forms update? Are you able to attach a repro project to the report so we can investigate if there is another reason you are hitting this condition? When providing the requested information, please set the report's status back to NEW otherwise we may close this issue after 30 days of no response. Thanks!
Comment 3 giedrius 2017-07-07 11:53:34 UTC
It happens around 5% of the time when popping the page. An idea how to reproduce would be to do this:

1. Have a main page with a button to push another page on the navigation stack.
2. Have the another page contain scroll view with stack layout with labels. Better to reuse the same page rather than keeping creating new one all the time.
3. Periodically, every 20-50ms append a new label to a stack layout in a scroll view, and request scrolling to the new item, all the time, regardless if the scroll view page is currently shown or not.
4. Rather quickly, as a user, keep clicking 'open another page' and back buttons, the crash should occur at some point.

I'll see if I can find some time next week to create a sample for this crash.

Anyway, inspecting the logic in OnScrollToRequested should also reveal the possibility of the race condition - the dispose might be invoked while in the 'while (IsLayoutRequested)' loop, as it contains 'await', which delays continuation of the execution, in the mean time allowing other code to run (such as back button getting pressed / handling OnDisappeared event, in turn disposing the renderer). The _isAttached gets checked only at the beginning of the function, so when the Task.Delay is finished, the ScrollViewRenderer may already have been disposed, and would attempt to access its properties while in the disposed state, producing the crash.

Setting to 'NEW' for now, if this additional information is not helpful in reproducing / fixing the issue, let me know, I will do my best to get the sample for you. :)
Comment 4 John Hardman 2018-02-07 10:48:58 UTC
I am also seeing this, using Xamarin.Forms 2.4.0.38779 
As a debugging/testing aid, I added the ability to navigate through all of the app's pages with one button press, pushing each page and popping it again, with delays between operations. On UWP this works fine, but on Android I see the exception shown below every time. The Android device is slower, so it's likely that I am popping pages whilst a scroll operation is happening or about to happen. That's something a human user could do too, so this is a problem that needs to be handled, even if it's just a case of silently swallowing the exception in the ScrollViewRenderer (I cannot believe I just suggested silently swallowing an exception...)


Xamarin caused by: android.runtime.JavaProxyThrowable: System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Xamarin.Forms.Platform.Android.ScrollViewRenderer'.
  at Java.Interop.JniPeerMembers.AssertSelf (Java.Interop.IJavaPeerable self) [0x00029] in <438784097c4b4b56a7da6ca9301bc3c6>:0 
  at Java.Interop.JniPeerMembers+JniInstanceMethods.InvokeVirtualBooleanMethod (System.String encodedMember, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) [0x00000] in <438784097c4b4b56a7da6ca9301bc3c6>:0 
  at Android.Views.View.get_IsLayoutRequested () [0x0000a] in <50fac8ede6e6437da7c8d41a391fc2c8>:0 
  at Xamarin.Forms.Platform.Android.ScrollViewRenderer+<OnScrollToRequested>d__57.MoveNext () [0x000ce] in D:\agent\_work\1\s\Xamarin.Forms.Platform.Android\Renderers\ScrollViewRenderer.cs:301 
--- End of stack trace from previous location where exception was thrown ---
  at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () [0x0000c] in <657aa8fea4454dc898a9e5f379c58734>:0 
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__6_0 (System.Object state) [0x00000] in <657aa8fea4454dc898a9e5f379c58734>:0 
  at Android.App.SyncContext+<>c__DisplayClass2_0.<Post>b__0 () [0x00000] in <50fac8ede6e6437da7c8d41a391fc2c8>:0 
  at Java.Lang.Thread+RunnableImplementor.Run () [0x00008] in <50fac8ede6e6437da7c8d41a391fc2c8>:0 
  at Java.Lang.IRunnableInvoker.n_Run (System.IntPtr jnienv, System.IntPtr native__this) [0x00008] in <50fac8ede6e6437da7c8d41a391fc2c8>:0 
  at (wrapper dynamic-method) System.Object:b0e5ac9d-ea9c-49a6-8c40-e1ca4f326b6c (intptr,intptr)
	at mono.java.lang.RunnableImplementor.n_run(Native Method)
	at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30)
	at android.os.Handler.handleCallback(Handler.java:739)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:148)
	at android.app.ActivityThread.main(ActivityThread.java:7325)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)
Comment 5 John Hardman 2018-02-14 10:05:14 UTC
Just to confirm that I can reproduce this manually, rather than as part of automated test.

With a page that in OnAppearing() automatically scrolls to a particular View (e.g. to show the first Entry on a page), do whatever option results in that page appearing, but then hit the Back button whilst the scroll is still happening.