Bug 22706 - Device.StartTimer has inconsistent behavior across platforms
Summary: Device.StartTimer has inconsistent behavior across platforms
Status: RESOLVED FIXED
Alias: None
Product: Forms
Classification: Xamarin
Component: Forms ()
Version: 1.2.2
Hardware: PC Windows
: Normal normal
Target Milestone: 1.3.5
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2014-09-05 14:56 UTC by Oren Novotny
Modified: 2016-09-23 22:28 UTC (History)
10 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 on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Oren Novotny 2014-09-05 14:56:31 UTC
Please see this thread for the details:
http://forums.xamarin.com/discussion/comment/74535

Bottom line is that invoking Device.StartTimer has different behavior on different platforms and that's not ideal. What it should do is be callable from any thread and invoke the callback on the UI/Main thread. That's the "hard" part, otherwise we could use a Timer/Task.Delay.

Current observed behavior:
iOS: If called from a UI thread, callbacks are on UI. If called from background thread, then ticks not on UI thread
WP8: Cannot be called at all from a background thread. Cross-thread exception is thrown from DispatcherTimer because wrong ctor overload is used.
Android: System.Threading.Timer is used, so can be called from any thread but callbacks aren't tied to the activity's thread.

Please fix this to have the same behavior across all platforms.
Comment 1 Narcís Calvet 2014-12-04 10:36:00 UTC
This also means that loading a View, which calls Device.StartTimer and consumes the callback event, from a NavigationPage and pressing the back button makes the view crash in Android with this error messsage:

Unhandled Exception:

System.ArgumentException: 'jobject' must not be IntPtr.Zero.

Parameter name: jobject

I'll try to arrange a simple repro project if I find some time for it.
Comment 2 Adam Kemp 2015-01-19 12:41:40 UTC
Any update on how this will be handled? I consider this a serious problem with the timer API that makes it impractical to actually use.
Comment 3 Jason Smith [MSFT] 2015-02-15 03:27:20 UTC
Should be fixed in 1.3.5-pre1
Comment 4 Adam Kemp 2015-02-15 12:58:29 UTC
Thanks, Jason! Could you describe the new behavior?
Comment 5 Jason Smith [MSFT] 2015-02-17 13:24:47 UTC
The timer now comes back on the main thread.
Comment 6 Adam Kemp 2015-02-17 14:33:56 UTC
I'm looking at the new code, and it doesn't look correct to me. This is what I see in Assembly Browser (from Xamarin.Forms.Platform.Android):

public void StartTimer(TimeSpan interval, Func<bool> callback)
{
	Timer timer = null;
	TimerCallback callback2 = delegate(object o)
	{
		this.BeginInvokeOnMainThread(delegate
		{
			if (callback.Invoke())
			{
				return;
			}
			timer.Dispose();
		});
	};
	timer = new Timer(callback2, null, interval, interval);
}

Consider the case where the time it takes to run that callback is longer than the timer interval. Since the Threading.Timer class runs in its own thread, and your callback doesn't block, it could end up calling the callback twice. If that happens you could also end up calling Dispose() twice on the underlying Timer object. Part of the problem with Threading.Timer is that it is difficult to stop without race conditions, and I believe this implementation has exactly that problem.

The preferable alternative is to use each platform's run-loop-aware timer mechanisms. On Android that would look something like this:

public void StartTimer(TimeSpan interval, Func<bool> callback)
{
    _handler = new Handler(Looper.MainLooper);
    _callback = callback;
    _interval = interval;
    _handler.PostDelayed(
        action: OnTick,
        delayMillis: interval.TotalMilliseconds);
}

private void OnTick()
{
    if (_callback)
    {
        StartTimer(_interval);
    }
    else
    {
        _handler.Dispose();
        _handler = null;
    }
}

Obviously since I added some fields there you would really make a new timer class that encapsulates this and just create it and start it in StartTimer, but hopefully you get the idea. Now the timer doesn't use a thread, is automatically invoked on the UI thread, and can be stopped synchronously without any race conditions.

NSTimer (iOS) and PlatformDispatcherTimer (Windows Phone) have similar semantics with different APIs. You create the timer, it fires on the UI thread, and you can stop it from the UI thread without risking another iteration firing.

Nearly every time I see someone use Threading.Timer in code it turns out that they have a bug in their implementation. I strongly encourage you to revisit this again and avoid that class. It might end up being more code, but the result will be much safer.
Comment 7 adrianknight89 2016-09-23 22:28:01 UTC
See https://github.com/xamarin/Xamarin.Forms/pull/374