From 6c16c13ed185acf16dc8f1c6ba458f1e59328e84 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 27 Oct 2012 13:21:26 +0200 Subject: [PATCH] Fix w32 implementation of itimers: overflow and ITIMER_PROF. Avoid overflow in w32 implementation of interval timers. When possible, for ITIMER_PROF count only times the main thread actually executes. src/w32proc.c : 'expire' and 'reload' are now ULONGLONG types. Likewise for all the other data which was previously clock_t. (GetThreadTimes_Proc): New typedef. (w32_get_timer_time): New function, returns a suitable time value for the timer. (timer_loop): Enter critical section when accessing ULONGLONG values of the itimer_data struct, as these accesses are no longer atomic. Call 'w32_get_timer_time' instead of 'clock'. (init_timers): Initialize s_pfn_Get_Thread_Times. (start_timer_thread): Don't assign itimer->caller_thread here. (getitimer): Assign itimer->caller_thread here. (setitimer): Always call getitimer to get the value of ticks_now. --- src/ChangeLog | 19 ++++++ src/w32proc.c | 177 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 147 insertions(+), 49 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index bf519556334..2efebc6754d 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,22 @@ +2012-10-27 Eli Zaretskii + + Avoid overflow in w32 implementation of interval timers. When + possible, for ITIMER_PROF count only times the main thread + actually executes. + * w32proc.c : 'expire' and 'reload' are now + ULONGLONG types. Likewise for all the other data which was + previously clock_t. + (GetThreadTimes_Proc): New typedef. + (w32_get_timer_time): New function, returns a suitable time value + for the timer. + (timer_loop): Enter critical section when accessing ULONGLONG + values of the itimer_data struct, as these accesses are no longer + atomic. Call 'w32_get_timer_time' instead of 'clock'. + (init_timers): Initialize s_pfn_Get_Thread_Times. + (start_timer_thread): Don't assign itimer->caller_thread here. + (getitimer): Assign itimer->caller_thread here. + (setitimer): Always call getitimer to get the value of ticks_now. + 2012-10-26 Eli Zaretskii * w32fns.c (w32_wnd_proc) : Don't enable tracking of diff --git a/src/w32proc.c b/src/w32proc.c index 57b3860cb76..fe3bbde167c 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -244,28 +244,89 @@ setpgrp (int pid, int gid) the thread calls the appropriate signal handler when the timer expires, after stopping the thread which installed the timer. */ -/* FIXME: clock_t counts overflow after 49 days, need to handle the - wrap-around. */ struct itimer_data { - clock_t expire; - clock_t reload; + ULONGLONG expire; + ULONGLONG reload; int terminate; int type; HANDLE caller_thread; HANDLE timer_thread; }; -static clock_t ticks_now; +static ULONGLONG ticks_now; static struct itimer_data real_itimer, prof_itimer; -static clock_t clocks_min; +static ULONGLONG clocks_min; /* If non-zero, itimers are disabled. Used during shutdown, when we delete the critical sections used by the timer threads. */ static int disable_itimers; static CRITICAL_SECTION crit_real, crit_prof; +/* GetThreadTimes is not available on Windows 9X and posibly also on 2K. */ +typedef BOOL (WINAPI *GetThreadTimes_Proc) ( + HANDLE hThread, + LPFILETIME lpCreationTime, + LPFILETIME lpExitTime, + LPFILETIME lpKernelTime, + LPFILETIME lpUserTime); + +static GetThreadTimes_Proc s_pfn_Get_Thread_Times; + +/* Return a suitable time value, in 1-ms units, for THREAD, a handle + to a thread. If THREAD is NULL or an invalid handle, return the + current wall-clock time since January 1, 1601 (UTC). Otherwise, + return the sum of kernel and user times used by THREAD since it was + created, plus its creation time. */ +static ULONGLONG +w32_get_timer_time (HANDLE thread) +{ + ULONGLONG retval; + int use_system_time = 1; + + if (thread && thread != INVALID_HANDLE_VALUE + && s_pfn_Get_Thread_Times != NULL) + { + FILETIME creation_ftime, exit_ftime, kernel_ftime, user_ftime; + ULARGE_INTEGER temp_creation, temp_kernel, temp_user; + + if (s_pfn_Get_Thread_Times (thread, &creation_ftime, &exit_ftime, + &kernel_ftime, &user_ftime)) + { + use_system_time = 0; + temp_creation.LowPart = creation_ftime.dwLowDateTime; + temp_creation.HighPart = creation_ftime.dwHighDateTime; + temp_kernel.LowPart = kernel_ftime.dwLowDateTime; + temp_kernel.HighPart = kernel_ftime.dwHighDateTime; + temp_user.LowPart = user_ftime.dwLowDateTime; + temp_user.HighPart = user_ftime.dwHighDateTime; + retval = + temp_creation.QuadPart / 10000 + temp_kernel.QuadPart / 10000 + + temp_user.QuadPart / 10000; + } + else + DebPrint (("GetThreadTimes failed with error code %lu\n", + GetLastError ())); + } + + if (use_system_time) + { + FILETIME current_ftime; + ULARGE_INTEGER temp; + + GetSystemTimeAsFileTime (¤t_ftime); + + temp.LowPart = current_ftime.dwLowDateTime; + temp.HighPart = current_ftime.dwHighDateTime; + + retval = temp.QuadPart / 10000; + } + + return retval; +} + #define MAX_SINGLE_SLEEP 30 +/* Thread function for a timer thread. */ static DWORD WINAPI timer_loop (LPVOID arg) { @@ -275,12 +336,13 @@ timer_loop (LPVOID arg) CRITICAL_SECTION *crit = (which == ITIMER_REAL) ? &crit_real : &crit_prof; const DWORD max_sleep = MAX_SINGLE_SLEEP * 1000 / CLOCKS_PER_SEC; int new_count = 0; + HANDLE hth = (which == ITIMER_REAL) ? NULL : itimer->caller_thread; while (1) { DWORD sleep_time; signal_handler handler; - clock_t now, expire, reload; + ULONGLONG now, expire, reload; /* Load new values if requested by setitimer. */ EnterCriticalSection (crit); @@ -290,15 +352,14 @@ timer_loop (LPVOID arg) if (itimer->terminate) return 0; - if (itimer->expire == 0) + if (expire == 0) { /* We are idle. */ Sleep (max_sleep); continue; } - expire = itimer->expire; - if (expire > (now = clock ())) + if (expire > (now = w32_get_timer_time (hth))) sleep_time = expire - now; else sleep_time = 0; @@ -309,8 +370,11 @@ timer_loop (LPVOID arg) if (itimer->terminate) return 0; Sleep (max_sleep); + EnterCriticalSection (crit); expire = itimer->expire; - sleep_time = (expire > (now = clock ())) ? expire - now : 0; + LeaveCriticalSection (crit); + sleep_time = + (expire > (now = w32_get_timer_time (hth))) ? expire - now : 0; } if (itimer->terminate) return 0; @@ -320,13 +384,16 @@ timer_loop (LPVOID arg) /* Always sleep past the expiration time, to make sure we never call the handler _before_ the expiration time, always slightly after it. Sleep(5) makes sure we don't - hog the CPU by calling 'clock' with high frequency, and - also let other threads work. */ - while (clock () < expire) + hog the CPU by calling 'w32_get_timer_time' with high + frequency, and also let other threads work. */ + while (w32_get_timer_time (hth) < expire) Sleep (5); } - if (itimer->expire == 0) + EnterCriticalSection (crit); + expire = itimer->expire; + LeaveCriticalSection (crit); + if (expire == 0) continue; /* Time's up. */ @@ -353,19 +420,21 @@ timer_loop (LPVOID arg) ResumeThread (itimer->caller_thread); } - if (itimer->expire == 0) - continue; - /* Update expiration time and loop. */ EnterCriticalSection (crit); expire = itimer->expire; + if (expire == 0) + { + LeaveCriticalSection (crit); + continue; + } reload = itimer->reload; if (reload > 0) { - now = clock (); + now = w32_get_timer_time (hth); if (expire <= now) { - clock_t lag = now - expire; + ULONGLONG lag = now - expire; /* If we missed some opportunities (presumably while sleeping or while the signal handler ran), skip @@ -448,6 +517,15 @@ term_timers (void) void init_timers (void) { + /* GetThreadTimes is not avaiulable on all versions of Windows, so + need to probe for its availability dynamically, and call it + through a pointer. */ + s_pfn_Get_Thread_Times = NULL; /* in case dumped Emacs comes with a value */ + if (os_subtype != OS_9X) + s_pfn_Get_Thread_Times = + (GetThreadTimes_Proc)GetProcAddress (GetModuleHandle ("kernel32.dll"), + "GetThreadTimes"); + /* Make sure we start with zeroed out itimer structures, since dumping may have left there traces of threads long dead. */ memset (&real_itimer, 0, sizeof real_itimer); @@ -473,14 +551,6 @@ start_timer_thread (int which) return 0; /* Start a new thread. */ - if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (), - GetCurrentProcess (), &itimer->caller_thread, 0, - FALSE, DUPLICATE_SAME_ACCESS)) - { - errno = ESRCH; - return -1; - } - itimer->terminate = 0; itimer->type = which; /* Request that no more than 64KB of stack be reserved for this @@ -512,17 +582,16 @@ start_timer_thread (int which) int getitimer (int which, struct itimerval *value) { - volatile clock_t *t_expire; - volatile clock_t *t_reload; - clock_t expire, reload; + volatile ULONGLONG *t_expire; + volatile ULONGLONG *t_reload; + ULONGLONG expire, reload; __int64 usecs; CRITICAL_SECTION *crit; + struct itimer_data *itimer; if (disable_itimers) return -1; - ticks_now = clock (); - if (!value) { errno = EFAULT; @@ -535,8 +604,22 @@ getitimer (int which, struct itimerval *value) return -1; } - t_expire = (which == ITIMER_REAL) ? &real_itimer.expire: &prof_itimer.expire; - t_reload = (which == ITIMER_REAL) ? &real_itimer.reload: &prof_itimer.reload; + itimer = (which == ITIMER_REAL) ? &real_itimer : &prof_itimer; + + if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (), + GetCurrentProcess (), &itimer->caller_thread, 0, + FALSE, DUPLICATE_SAME_ACCESS)) + { + errno = ESRCH; + return -1; + } + + ticks_now = w32_get_timer_time ((which == ITIMER_REAL) + ? NULL + : itimer->caller_thread); + + t_expire = &itimer->expire; + t_reload = &itimer->reload; crit = (which == ITIMER_REAL) ? &crit_real : &crit_prof; EnterCriticalSection (crit); @@ -560,10 +643,11 @@ getitimer (int which, struct itimerval *value) int setitimer(int which, struct itimerval *value, struct itimerval *ovalue) { - volatile clock_t *t_expire, *t_reload; - clock_t expire, reload, expire_old, reload_old; + volatile ULONGLONG *t_expire, *t_reload; + ULONGLONG expire, reload, expire_old, reload_old; __int64 usecs; CRITICAL_SECTION *crit; + struct itimerval tem, *ptem; if (disable_itimers) return -1; @@ -573,26 +657,21 @@ setitimer(int which, struct itimerval *value, struct itimerval *ovalue) time we are called, measure the clock tick resolution. */ if (!clocks_min) { - clock_t t1, t2; + ULONGLONG t1, t2; - for (t1 = clock (); (t2 = clock ()) == t1; ) + for (t1 = w32_get_timer_time (NULL); + (t2 = w32_get_timer_time (NULL)) == t1; ) ; clocks_min = t2 - t1; } if (ovalue) - { - if (getitimer (which, ovalue)) /* also sets ticks_now */ - return -1; /* errno already set */ - } + ptem = ovalue; else - ticks_now = clock (); + ptem = &tem; - if (which != ITIMER_REAL && which != ITIMER_PROF) - { - errno = EINVAL; - return -1; - } + if (getitimer (which, ptem)) /* also sets ticks_now */ + return -1; /* errno already set */ t_expire = (which == ITIMER_REAL) ? &real_itimer.expire : &prof_itimer.expire; -- 2.39.2