Bug 3025 - Changing time problem with EventWaitHandle.WaitOne(timeout)
Summary: Changing time problem with EventWaitHandle.WaitOne(timeout)
Status: RESOLVED FIXED
Alias: None
Product: Runtime
Classification: Mono
Component: io-layer (show other bugs)
Version: 4.2.2 (C6SR1)
Hardware: All Linux
: Normal normal
Target Milestone: Future Cycle (TBD)
Assignee: Bugzilla
URL:
Depends on:
Blocks:
 
Reported: 2012-01-24 03:51 UTC by Mirko Wischer
Modified: 2017-07-11 22:07 UTC (History)
15 users (show)

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


Attachments
misc.c changed from gettimeofday to clock_gettime(CLOCK_MONOTONIC,..) (902 bytes, patch)
2012-01-24 03:51 UTC, Mirko Wischer
Details
handles.c patch for using CLOCK_MONOTONIC for pthread_cond (473 bytes, patch)
2012-01-24 03:52 UTC, Mirko Wischer
Details
Updated diff (3.26 KB, patch)
2013-07-23 08:56 UTC, Dick Porter
Details
Small test case for the problem on linux (1.19 KB, text/plain)
2015-12-22 13:19 UTC, matthias.schnelte
Details
Patch to work with Mono 4.4.0.122 (3.09 KB, patch)
2016-08-17 06:53 UTC, Craig McQueen
Details


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 GitHub or Developer Community 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 Mirko Wischer 2012-01-24 03:51:00 UTC
Created attachment 1257 [details]
misc.c changed from gettimeofday to clock_gettime(CLOCK_MONOTONIC,..)

I am using named events and EventWaitHandle.WaitOne(time) to wait for an event, (timeout is often a longer timespan).
But if someone changes the clock during the waiting time WaitOne aborts immediately. This only occurs when I'm using named events.

I've analyzed the code a bit: 
In the io-layer  the WaitForSingleObjectEx method uses _wapi_calc_timeout (from misc.c) to compute an absolute time from the given timeout, but the absolute timeout is computed using “gettimeofday”. That will cause problems if the system clock changes during checking (named events spin through fake timeouts until the absolute timeout seems to be reached). 
Unnamed events also use the _wapi_calc_timeout method to compute the absolute timeout. This timeout is then used in mono_cond_timedwait (but this only wraps “normal” pthread conditions). 

So I prepared a patch (see attached files) using clock_gettime(CLOCK_MONOTONIC,..) instead of gettimeofday, but this also requires
a change for unnamed events, because the pthreads condition needs to know what kind of clock provides the absolute timeout.
Comment 1 Mirko Wischer 2012-01-24 03:52:46 UTC
Created attachment 1258 [details]
handles.c patch for using CLOCK_MONOTONIC for pthread_cond
Comment 2 Mirko Wischer 2012-03-20 09:30:28 UTC
It seems that this could also happen using unnamed events e.g. in a multi-threaded application. 
But this is unconfirmed, need to check this.
Comment 3 Miguel de Icaza [MSFT] 2012-09-02 13:39:06 UTC
This solution seems to be suitable for some versions of Linux.

Please attach a patch with the approprieate configure probing.
Comment 4 Dick Porter 2013-07-23 08:56:41 UTC
Created attachment 4398 [details]
Updated diff

Here's an updated diff.  It doesn't need autoconf probes, as the POSIX feature test macros cover this.

I've tested on MacOSX 10.8 and Linux.

OK to commit to 2-10 and master branches?
Comment 5 matthias.schnelte 2015-12-22 13:19:21 UTC
Created attachment 14376 [details]
Small test case for the problem on linux

Is there any reason why the proposed patch has not been applied yet?
I ran into the same bug in a project and applying the proposed patch solved the problem on the latest master branch.

The patch is however no long up-to-date. The changes for misc.c have now to be applied to mono-os-mutex.h in the function mono_os_cond_timedwait

I did attach a small test program for Linux that shows the bug.
Run the program with sudo to be able to change the system date.

After the time has been set back into the past the waitone will wait for 31 sec instead of 1 and the timer also is not called for 30 seconds.
Comment 6 Craig McQueen 2016-04-01 00:56:55 UTC
I've come across this issue in an embedded Linux system. Clock change by NTP messes up the timers. This seems like a crucial issue to fix, so I'm surprised it hasn't been fixed in Mono yet.

I'm using meta-mono for Yocto. I was able to patch version 4.2.2.30 (the current default in meta-mono) with a patch based on the attached patch. Although mono/io-layer/mono-mutex.c doesn't exist in 4.2.2.30, it was a comment-only patch anyway.

I had a look at 4.3.2.467 which is supported by the current meta-mono, but not the default. As Matthias said, in 4.3.2.467 the patch would need to be reworked since _wapi_calc_timeout() in mono/io-layer/misc.c no longer exists, and the functionality is moved to mono_os_cond_timedwait() in mono/utils/mono-os-mutex.h.
Comment 7 Craig McQueen 2016-04-11 00:40:59 UTC
Would it help if someone would provide an up-to-date patch? If so, what version should the patch file be against? Master branch, latest release, other?
Comment 8 Craig McQueen 2016-07-05 02:15:55 UTC
Could we please give this bug some attention. It's a corner-case, but the effects could be bad on systems that experience it.
Comment 9 François MAYIS 2016-08-08 17:13:03 UTC
Exactly same problem encountered and quite hard to find the cause.

Is there any plan to fix this bug on the master branch?
Is there any reason why it is not included in the master branch?
Thanks
Comment 10 Craig McQueen 2016-08-17 06:53:06 UTC
Created attachment 17070 [details]
Patch to work with Mono 4.4.0.122

I'm attaching a patch for Mono 4.4.0.122 (which is the version I'm using with the meta-mono layer for Yocto). It is not the latest, but it is updated according to the code restructuring which has occurred since version 4.2.2.30.
Comment 11 Craig McQueen 2016-08-18 23:44:01 UTC
I've submitted a pull request in GitHub for the Mono project, to fix this in the master branch.
https://github.com/mono/mono/pull/3414
Comment 12 Will McFarlane 2016-08-23 07:43:42 UTC
Yep - we are experiencing this very same problem on our Embedded Linux system. It started manifesting with Mono 4.4.0.
Comment 13 Will McFarlane 2016-08-25 11:13:18 UTC
I have applied Craig's patch (thanks) as listed in attachment 17070 [details] and retested. Unfortunately, this patch did not work for us.

Our instrument offers the user the ability to change the Date/Time. If the user does this, then a number of features all fail. These failures have been traced to the use of ''System.Threading.WaitHandle.WaitOne(Int32)'' which appears to make use of CLOCK_REALTIME in Mono. I was hoping that CLOCK_MONOTONIC would be used.
Comment 14 Alex Rønne Petersen 2016-08-29 06:16:31 UTC
Hi Will,

As far as I can tell, we end up using mono_os_cond_timedwait () when doing a WaitHandle.WaitOne () from managed with Mono master, so Craig's patch should be good. Which Mono version are you on?
Comment 15 Will McFarlane 2016-08-29 16:31:50 UTC
Hi Alex,

Understood. 

Specifically, we moved to 4.4.0.182 from 3.2.8 back in May '16 and have been using it on our Debian Linux (Embedded) system. It's only relatively recently that we got to the bottom of this particular issue. It's a PITA to diagnose. 

Today, we have decided to roll to 4.4.2.11 to pick up https://bugzilla.xamarin.com/show_bug.cgi?id=42688 as we just discovered that this affects some of our longer timeouts related to programming very large hardware devices.
Comment 16 Craig McQueen 2016-08-29 22:57:24 UTC
Will, I'm puzzled if the patch doesn't fix this bug for you.
Comment 17 Brandon White 2017-01-25 16:49:45 UTC
This issue is affecting my application in Mono 4.8.  I'd really like to see it resolved.
Comment 18 Craig McQueen 2017-05-01 04:54:02 UTC
It seems this has been fixed in GitHub pull request 4446.
https://github.com/mono/mono/pull/4446

However it looks as though that hasn't gone into a release yet (I've searched git history of 5.0.0.78 and 4.8.0.524).
Comment 19 James B 2017-05-17 12:05:09 UTC
In the patch, I don't see a change to "_wapi_handle_spin" method. The current code is using nanosleep, should this change to clock_nanosleep, to ENSURE it is using the monotonic clock? 

Also, for "_wapi_handle_init" the clock type is not specified, is this correct?
Comment 20 Rodrigo Kumpera 2017-07-11 22:07:35 UTC
Fix shipped with mono 5.2