Re: [PATCH glibc-2.44] Fix __pthread_timedblock for hurd

Hi Samuel,

OK, maybe you are correct regarding the abstime parameter.
However, the man page for pthread_cond_init(3) says:

The abstime parameter specifies an absolute time, with the same origin 
as time(2) and gettimeofday(2): an abstime of 0 corresponds to 00:00:00 
GMT, January 1, 1970.

This is not consistent with CLOCK_MONOTONIC on GNU/Hurd.
Could this be causing a misalignment in java? I am getting hangs on 
pthread_cond_timedwait().

Damien

On 18/8/26 10:07 am, Samuel Thibault wrote:
> Hello,
>
> Damien Zammit, le lun. 17 août 2026 23:49:24 +0000, a ecrit:
>> When clock_id is CLOCK_MONOTONIC, there is an assumption
>> that abstime parameter is actually a relative time
> Where is such assumption?
>
> pthread_cond_timedwait always take an absolute time.
>
>> but the code was not making that assumption and getting the timeout
>> wrong.
>>
>> This is not tested, but according to OpenJDK 1.7.0 sources:
>>
>>    if ((status = pthread_condattr_setclock(_condattr, CLOCK_MONOTONIC)) != 0) {
>>      if (status == EINVAL) {
>>        warning("Unable to use monotonic clock with relative timed-waits" \
>>                " - changes to the time-of-day clock may have adverse affects");
>>
>> So there are sources out there making this assumption.
> I believe the "relative" word above is misunderstood. It doesn't mean
> that the value is relative to "time now", but that it is relative to
> e.g. the machine bootup, and not an absolute time which will be affected
> by time-of-day changes. That's exactly what selecting CLOCK_MONOTONIC
> means: get something that monitors actual time that passes, and not the
> wall-clock time.
>
> Even with CLOCK_MONOTONIC, pthread_cond_timedwait should be given values
> akin to what clock_gettime() returns.
>
> Samuel
>
>> ---
>>   .../hurd-pt-timedblock-relative.diff          | 54 +++++++++++++++++++
>>   debian/patches/series                         |  1 +
>>   2 files changed, 55 insertions(+)
>>   create mode 100644 debian/patches/hurd-i386/hurd-pt-timedblock-relative.diff
>>
>> diff --git a/debian/patches/hurd-i386/hurd-pt-timedblock-relative.diff b/debian/patches/hurd-i386/hurd-pt-timedblock-relative.diff
>> new file mode 100644
>> index 00000000..a7819ce3
>> --- /dev/null
>> +++ b/debian/patches/hurd-i386/hurd-pt-timedblock-relative.diff
>> @@ -0,0 +1,54 @@
>> +--- a/sysdeps/mach/htl/pt-timedblock.c
>> ++++ b/sysdeps/mach/htl/pt-timedblock.c
>> +@@ -40,23 +40,37 @@
>> +   mach_msg_timeout_t timeout;
>> +   struct timespec now;
>> +
>> +-  /* We have an absolute time and now we have to convert it to a
>> +-     relative time.  Arg.  */
>> +
>> +-  err = __clock_gettime (clock_id, &now);
>> +-  assert (!err);
>> ++  if (clock_id == CLOCK_MONOTONIC)
>> ++    {
>> ++      /* abstime param should be a relative time */
>> ++      if (abstime->tv_sec < 0
>> ++          || (abstime->tv_sec == 0 && abstime->tv_nsec < 0))
>> ++        return ETIMEDOUT;
>> +
>> +-  if (now.tv_sec > abstime->tv_sec
>> +-      || (now.tv_sec == abstime->tv_sec && now.tv_nsec > abstime->tv_nsec))
>> +-    return ETIMEDOUT;
>> +-
>> +-  timeout = (abstime->tv_sec - now.tv_sec) * 1000;
>> +-
>> +-  if (abstime->tv_nsec >= now.tv_nsec)
>> +-    timeout += (abstime->tv_nsec - now.tv_nsec + 999999) / 1000000;
>> ++      timeout = abstime->tv_sec * 1000 + (abstime->tv_nsec + 999999) / 1000000;
>> ++    }
>> +   else
>> +-    /* Need to do a carry.  */
>> +-    timeout -= (now.tv_nsec - abstime->tv_nsec + 999999) / 1000000;
>> ++    {
>> ++      /* We have an absolute time and now we have to convert it to a
>> ++         relative time.  This codepath could be avoided by using CLOCK_MONOTONIC
>> ++         and passing a relative time for the abstime parameter. */
>> ++
>> ++      err = __clock_gettime (clock_id, &now);
>> ++      assert (!err);
>> ++
>> ++      if (now.tv_sec > abstime->tv_sec
>> ++          || (now.tv_sec == abstime->tv_sec && now.tv_nsec > abstime->tv_nsec))
>> ++        return ETIMEDOUT;
>> ++
>> ++      timeout = (abstime->tv_sec - now.tv_sec) * 1000;
>> ++
>> ++      if (abstime->tv_nsec >= now.tv_nsec)
>> ++        timeout += (abstime->tv_nsec - now.tv_nsec + 999999) / 1000000;
>> ++      else
>> ++        /* Need to do a carry.  */
>> ++        timeout -= (now.tv_nsec - abstime->tv_nsec + 999999) / 1000000;
>> ++    }
>> +
>> +   err = __mach_msg (&msg, MACH_RCV_MSG | MACH_RCV_TIMEOUT | MSG_OPTIONS, 0,
>> + 		    sizeof msg, thread->wakeupmsg.msgh_remote_port,
>> diff --git a/debian/patches/series b/debian/patches/series
>> index f4346bf6..4d9ea389 100644
>> --- a/debian/patches/series
>> +++ b/debian/patches/series
>> @@ -41,6 +41,7 @@ hurd-i386/submitted-bind_umask2.diff
>>   hurd-i386/tg-bootstrap.diff
>>   hurd-i386/local-no_unsupported_ioctls.diff
>>   hurd-i386/local-stack_chk_guard.diff
>> +hurd-i386/hurd-pt-timedblock-relative.diff
>>
>>   i386/local-biarch.diff
>>   i386/unsubmitted-quiet-ldconfig.diff
>> --
>> 2.51.0
>>
>>
>>
> --
> Samuel
> War doesn't prove who's right, just who's left.