Problems with Tcl_SetTimeProc and a threaded 8.5.9 interpreter.

[Joe Keen]

I'm working on a system where the system clock can be adjusted
arbitrarily. To prevent after commands from stalling due to system
time changes we use Tcl_SetTimeProc to replace the Tcl_GetTime method
with one referencing a monotonic clock. This works as expected in
8.5.9 provided we haven't compiled with --enable-threads. If we have
compiled with --enable-threads we see some odd behaviour depending on
the after syntax we use.

------------------------------------------------------------
load "libTimeProc.so" TimeProc

proc x {} {
puts A
after 1000
x
}

x
vwait forever
-------------------------------------------------------------

The preceding script works as expected with or without threads
enabled.

------------------------------------------------------------
load "libTimeProc.so" TimeProc

proc x {} {
puts A
after 1000 {x}
}

x
vwait forever
-------------------------------------------------------------

This script fails in interesting ways. Without adjusting the time the
after fires every one second as expected. If I put the system clock
back a second the afters are processed every two seconds. If I set
the clock back an additional second the afters are processed at three
second intervals. If I set the clock back an hour the afters appear
to stall but I'd bet they're just going to show at one hour intervals.

This behaviour seems consistent on both 8.5.9 and 8.5.6 threaded
interpreters. Has anyone else had trouble replacing the standard
Tcl_GetTime function?

-------------------------------------------------------------
libTimeProc.so:

#include <ctime>
#include <sys/time.h>
#include <tcl.h>

//------------------------------------------------------------------------

static void GetTime (Tcl_Time*, ClientData);
static int EpochTime(ClientData, Tcl_Interp*, int, Tcl_Obj *const[]);

static struct timeval system0 = { 0 };
static struct timespec monotonic0 = { 0 };

//------------------------------------------------------------------------

/*
* Extension initialization. Base line time values are recorded once
* per process.
*/
EXTERN int Timeproc_Init(Tcl_Interp* interp) {

if (system0.tv_sec == 0) {

gettimeofday(&system0, 0);
clock_gettime(CLOCK_MONOTONIC, &monotonic0);

Tcl_ScaleTimeProc* scaleProcPtr;
ClientData cd;

Tcl_QueryTimeProc(NULL, &scaleProcPtr, &cd);
Tcl_SetTimeProc(GetTime, scaleProcPtr, cd);

}

Tcl_CreateObjCommand(interp, "EpochTime", EpochTime, 0, 0);

return TCL_OK;
}

//------------------------------------------------------------------------

/*
* Extension initialization for 'safe' interpreters. No distinction
is
* made for the purposes of this extension.
*/
EXTERN int Timeproc_SafeInit(Tcl_Interp* interp) {

return Timeproc_Init(interp);
}

//------------------------------------------------------------------------

void GetTime(Tcl_Time* timebuf, ClientData cd) {

struct timespec now;

clock_gettime(CLOCK_MONOTONIC, &now);

// system0 + (now - monotonic0)

now.tv_sec -= monotonic0.tv_sec;
now.tv_nsec -= monotonic0.tv_nsec;

if (now.tv_nsec < 0) {
now.tv_sec--;
now.tv_nsec += 1000000000;
}

timebuf->sec = system0.tv_sec + now.tv_sec;
timebuf->usec = system0.tv_usec + now.tv_nsec / 1000;

if (timebuf->usec >= 1000000) {
timebuf->sec++;
timebuf->usec -= 1000000;
}
}

//------------------------------------------------------------------------

/*
* Return the epoch time in seconds from 1970-01-01 00:00.00. This
* function is provided to replace [clock seconds] calls to obtain
* the real world date/time.
*/
int EpochTime(ClientData cd, Tcl_Interp *interp, int objc,
Tcl_Obj *const objv[]) {

struct timeval tv;

gettimeofday(&tv, 0);

Tcl_Obj* result = Tcl_NewLongObj((long) tv.tv_sec);
Tcl_SetObjResult(interp, result);

return TCL_OK;
}

[Joe Keen]

The problem appears to be in the Tcl_ConditionWait function. Despite
the use of Tcl_GetTime within the function the pthread_cond_timedwait
call is waiting on an absolute time and is sensitive to changes in the
system clock. In the case of the code posted previously the monotonic
clock was initially set to the system clock time. The increasing
delta between the system clock and the monotonic clock as the system
clock was changed causes the absolute time, based on the monotonic
clock, passed to pthread_cond_timedwait to be incorrect.

[Alexandre Ferrieux]

Yes. For a bit more on the implications of fixing this, see TIP#302:

http://www.tcl.tk/cgi-bin/tct/tip/302.html

Bottom line: it's more a problem of compatibility than of
implementation. A clock-insensitive variant of [after] would simply
have to be named differently (or use an option).

-Alex

[Uwe Klein]

Alexandre Ferrieux wrote:
> Bottom line: it's more a problem of compatibility than of
> implementation. A clock-insensitive variant of [after] would simply
> have to be named differently (or use an option).
>
> -Alex

after is a "timer"
form follows function in this case function should derive naming.
Thus and imho after should not be influenced by clock shifts.

for the clock case one would need a do@ command ( utilising the
clock scan dsl ;-)

top up with better introspection and "kick on" for after and do@.

uwe

[Joe Keen]

On Jun 15, 1:46 pm, Alexandre Ferrieux <alexandre.ferri...@gmail.com>
wrote:

The surprising part, from our perspective, is that using
Tcl_SetTimeProc is insufficient in a threaded interpreter. In a non-
threaded interpreter it works fine and that was, unfortunately, what
we accidentally tested against originally. Our main use case also
happened to work fine, we have a file descriptor that is active every
40 milliseconds so the pthread call is continually interrupted and the
after calls don't stall for noticeable lengths of time. We only
noticed the problem several months after putting the replacement
GetTime function in place.

Do you recommend against attempting to replace the GetTime function in
a threaded environment? Is there any other way we could play with the
clock, or even scale time, and still have it function correctly?

[Alexandre Ferrieux]

On Jun 16, 12:21 am, Joe Keen <joek...@gmail.com> wrote:
>
> Do you recommend against attempting to replace the GetTime function in
> a threaded environment?  Is there any other way we could play with the
> clock, or even scale time, and still have it function correctly?

(sorry for taking so long to come back to you ...)

Oh, you've found something !

I eventually managed to test by myself, and strace shows that the
reason of the problem is something using CLOCK_REALTIME (instead of
CLOCK_MONOTONIC) under the cover. A bit of gdb syscall catching shows
that the pthread_cond_timedwait function is the culprit. Then a bit of
googling shows:

http://stackoverflow.com/questions/3006259/what-time-function-do-i-need-to-use-with-pthread-cond-timedwait

Bottom line: pthread_cond_timedwait takes an absolute target time, and
when the selected clock is REALTIME you get essentially back [after]'s
default behavior. Of course, this happens only in threaded builds,
because unthreaded ones don't use pthread_conds.

The cure, then, is (a) to use specific attributes when creating the
pthread_cond, and (b) to modify the target time computation so that it
is expressed in the proper clock. This all happens in
Tcl_ConditionWait, attaching the patch below. You'll note that you
need both your extension *and* the patch -- unfortunately, the
mismatch in spirit between pthread's and Tcl's ideas of time makes it
impossible to do it purely as an extension...
(of course you can turn it all in a single patch, putting your
extension's code in tclMain.c -- that's what I did to test it).

Doing this, we get Tcl timers that are truly relative, ticking away
regardless of system clock tweaks: congrats !!!
What's funny when I do this is that all my IceWm gadgets suddenly
freeze, but no longer the Tcl app :D

Many thanks for finding this. You may want to file a bugreport, since
it is a fundamental limitation in the current "virtual time" support
that is everything but obvious...

-Alex

Index: unix/tclUnixThrd.c
===================================================================
--- unix/tclUnixThrd.c
+++ unix/tclUnixThrd.c
@@ -538,12 +538,22 @@
* Double check inside mutex to avoid race, then initialize
condition
* variable if necessary.
*/

if (*condPtr == NULL) {
+#ifdef TIME_MONOTONIC
+ pthread_condattr_t attr;
+ pthread_condattr_init(&attr);
+ pthread_condattr_setclock(&attr,CLOCK_MONOTONIC);
+#endif
pcondPtr = ckalloc(sizeof(pthread_cond_t));
+#ifdef TIME_MONOTONIC
+ pthread_cond_init(pcondPtr, &attr);
+ pthread_condattr_destroy(&attr);
+#else
pthread_cond_init(pcondPtr, NULL);
+#endif
*condPtr = (Tcl_Condition) pcondPtr;
TclRememberCondition(condPtr);
}
MASTER_UNLOCK;
}
@@ -550,21 +560,31 @@
pmutexPtr = *((pthread_mutex_t **)mutexPtr);
pcondPtr = *((pthread_cond_t **)condPtr);
if (timePtr == NULL) {
pthread_cond_wait(pcondPtr, pmutexPtr);
} else {
+#ifdef TIME_MONOTONIC
+ struct timespec pnow;
+#else
Tcl_Time now;
+#endif

/*
* Make sure to take into account the microsecond component of
the
* current time, including possible overflow situations. [Bug
#411603]
*/
-
+#ifdef TIME_MONOTONIC
+ clock_gettime(CLOCK_MONOTONIC, &pnow);
+ ptime.tv_sec = timePtr->sec + pnow.tv_sec +
+ (timePtr->usec + pnow.tv_nsec/1000) / 1000000;
+ ptime.tv_nsec = 1000 * ((timePtr->usec + pnow.tv_nsec/1000) %
1000000);
+#else
Tcl_GetTime(&now);
ptime.tv_sec = timePtr->sec + now.sec +
(timePtr->usec + now.usec) / 1000000;
ptime.tv_nsec = 1000 * ((timePtr->usec + now.usec) % 1000000);
+#endif
pthread_cond_timedwait(pcondPtr, pmutexPtr, &ptime);
}
}

/*