Problems with the new pthread clock implementations

* Problems with the new pthread clock implementations
@ 2020-11-21  6:59 Michael Kerrisk (man-pages)
  2020-11-21 17:54 ` Mike Crowe
  2020-11-21 19:50 ` Thomas Gleixner
  0 siblings, 2 replies; 7+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-11-21  6:59 UTC (permalink / raw)
  To: Mike Crowe; +Cc: Adhemerval Zanella, libc-alpha, Linux API

Hello Mike,

I've been taking a closer look at the the new pthread*clock*() APIs:
pthread_clockjoin_np()
pthread_cond_clockwait()
pthread_mutex_clocklock()
pthread_rwlock_clockrdlock()
pthread_rwlock_clockwrlock()
sem_clockwait()

I've noticed some oddities, and at least a couple of bugs.

First off, I just note that there's a surprisingly wide variation in
the low-level futex calls being used by these APIs when implementing
CLOCK_REALTIME support:

pthread_rwlock_clockrdlock()
pthread_rwlock_clockwrlock()
sem_clockwait()
pthread_cond_clockwait()
    futex(addr,
        FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 3,
        {abstimespec}, FUTEX_BITSET_MATCH_ANY)
    (This implementation seems to be okay)

pthread_clockjoin_np()
    futex(addr, FUTEX_WAIT, 48711, {reltimespec})
    (This is buggy; see below.)

pthread_mutex_clocklock()
    futex(addr, FUTEX_WAIT_PRIVATE, 2, {reltimespec})
    (There's bugs and strangeness here; see below.)

=== Bugs ===

pthread_clockjoin_np():
As already recognized in another mail thread [1], this API accepts any
kind of clockid, even though it doesn't support most of them.

A further bug is that even if CLOCK_REALTIME is specified,
pthread_clockjoin_np() sleeps against the CLOCK_MONOTONIC clock.
(Currently it does this for *all* clockid values.) The problem here is
that the FUTEX_WAIT operation sleeps against the CLOCK_MONOTONIC clock
by default. At the least, the FUTEX_CLOCK_REALTIME is required for
this case. Alternatively, an implementation using
FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME (like the first four
functions listed above) might be appropriate.

===

pthread_mutex_clocklock():
First of all, there's a small oddity. Suppose we specify the clockid
as CLOCK_REALTIME, and then while the call is blocked, we set the
clock realtime backwards. Then, there will be further futex calls to
handle the modification to the clock (and possibly multiple futex
calls if the realtime clock is adjusted repeatedly):

        futex(addr, FUTEX_WAIT_PRIVATE, 2, {reltimespec1})
        futex(addr, FUTEX_WAIT_PRIVATE, 2, {reltimespec2})
        ...

Then there seems to be a bug. If we specify the clockid as
CLOCK_REALTIME, and while the call is blocked we set the realtime
clock forwards, then the blocking interval of the call is *not*
adjusted (shortened), when of course it should be.

===

I've attached a couple of small test programs at the end of this mail.

Thanks,

Michael

[1] https://lore.kernel.org/linux-man/20201119120034.GA20599@mcrowe.com/

=============================================

/* t_pthread_clockjoin_np.c

   Copyright Michael Kerrisk, 2020. Licensed GPLv2+
*/
#define _GNU_SOURCE
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>

#define errExit(msg)    do { perror(msg); exit(EXIT_FAILURE); \
                        } while (0)

#define errExitEN(en, msg) \
                        do { errno = en; perror(msg); \
                             exit(EXIT_FAILURE); } while (0)

static void *
threadFunc(void *arg)
{
    printf("New thread started\n");
    sleep(2000);
    return NULL;
}

int
main(int argc, char *argv[])
{
    pthread_t t1;
    int nsecs;
    int s;

    nsecs = (argc > 1) ? atoi(argv[1]) : 10;

    s = pthread_create(&t1, NULL, threadFunc, (void *) 1);
    if (s != 0)
        errExitEN(s, "pthread_create");

    struct timespec ts, tsm1, tsm2;
    if (clock_gettime(CLOCK_MONOTONIC, &tsm1) == -1)
        errExit("clock_gettime");

    int clockid;
    clockid = CLOCK_REALTIME;
    if (clock_gettime(clockid, &ts) == -1)
        errExit("clock_gettime");

    ts.tv_sec += nsecs;
    printf("ts.tv_sec = %ld\n", ts.tv_sec);

    s = pthread_clockjoin_np(t1, NULL, clockid, &ts);
    if (s == 0) {
        printf("pthread_clockjoin_np() successfully joined\n");
    } else {
        if (s == ETIMEDOUT)
            printf("pthread_clockjoin_np() timed out\n");
        else
            errExitEN(s, "pthread_clockjoin_np");
    }

    if (clock_gettime(CLOCK_MONOTONIC, &tsm2) == -1)
        errExit("clock_gettime");

    printf("CLOCK_MONOTONIC diff = %ld secs\n", tsm2.tv_sec - tsm1.tv_sec);

    exit(EXIT_SUCCESS);
}

=============================================

/* t_pthread_mutex_clocklock.c

   Copyright Michael Kerrisk, 2020. Licensed GPLv2+
*/
#define _GNU_SOURCE
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>

#define errExit(msg)    do { perror(msg); exit(EXIT_FAILURE); \
                        } while (0)

#define errExitEN(en, msg) \
                        do { errno = en; perror(msg); \
                             exit(EXIT_FAILURE); } while (0)

static pthread_mutex_t mtx = PTHREAD_MUTEX_INITIALIZER;

static void *
threadFunc(void *arg)
{
    int s;

    printf("Thread created\n");
    s = pthread_mutex_lock(&mtx);
    if (s != 0)
        errExitEN(s, "pthread_mutex_lock");
    else
        printf("Thread 1 locked mutex\n");

    pause();
    return NULL;
}

int
main(int argc, char *argv[])
{
    pthread_t t1;
    int nsecs;
    int s;

    nsecs = (argc > 1) ? atoi(argv[1]) : 10;

    s = pthread_create(&t1, NULL, threadFunc, argv[1]);
    if (s != 0)
        errExitEN(s, "pthread_create");

    usleep(100000);

    struct timespec ts, tsm1, tsm2;
    if (clock_gettime(CLOCK_MONOTONIC, &tsm1) == -1)
        errExit("clock_gettime");

    int clockid;
    clockid = CLOCK_REALTIME;
    if (clock_gettime(clockid, &ts) == -1)
        errExit("clock_gettime");

    ts.tv_sec += nsecs;
    printf("ts.tv_sec = %ld\n", ts.tv_sec);

    s = pthread_mutex_clocklock(&mtx, clockid, &ts);
    if (s == 0) {
        printf("pthread_mutex_clocklock() got lock\n");
    } else {
        if (s == ETIMEDOUT)
            printf("pthread_mutex_clocklock() timed out\n");
        else
            errExitEN(s, "pthread_mutex_clocklock");
    }

    if (clock_gettime(CLOCK_MONOTONIC, &tsm2) == -1)
        errExit("clock_gettime");

    printf("CLOCK_MONOTONIC diff = %ld secs\n", tsm2.tv_sec - tsm1.tv_sec);

    exit(EXIT_SUCCESS);
}

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 7+ messages in thread