Menu ▾ ▴

#447 Wrong errno handling in vos_threadDelay() function

3.0.1.0
closed
2432 (1) 2434 (1)
2024-02-09
2024-02-01
No

I refer to the vos_threadDelay() function located in src/vos/posix/vos_thread.c.

This function shows several problems in version 3.0.0.0 and in the former version also. This was obviously known, at least ticket #345 suggests this. However thicket #345 did not resolve the root cause of the problem but introduced new ones.

The basic problem of vos_threadDelay() prior to version 2.1 was inappropriate handling of errno and not concurrent calls to nanosleep(). Please look at the code sippet below:

EXT_DECL VOS_ERR_T vos_threadDelay (
    UINT32 delay)
{
    ...
    wanted_delay.tv_sec     = delay / 1000000u;
    wanted_delay.tv_nsec    = (delay % 1000000) * 1000;
    do
    {
        pthread_testcancel();
        ret = nanosleep(&wanted_delay, &remaining_delay);
        if (ret == -1 && errno == EINTR)
        {
            wanted_delay = remaining_delay;
        }
    }
    while (errno == EINTR);

    return VOS_NO_ERR;
}

Whenever nanosleep() is interrupted by a signal, this code deadlocks. What happens is that the while loop will repeat, but errno will no more be reset to 0. This is beause "the value of errno is never set to zero by any system call or library function.", see https://man7.org/linux/man-pages/man3/errno.3.html
As a result the function deadlocks.

The correction in 2.1 did not resolve this but introduce new problems. The clock_nanosleep() function
does not set errno as nanosleep() does, see https://man7.org/linux/man-pages/man2/clock_nanosleep.2.html and https://man7.org/linux/man-pages/man2/nanosleep.2.html . This may not be obvious, but debugging shows exactly this behavior and glibc code (2.35) suggests this also, see https://elixir.bootlin.com/glibc/glibc-2.35/source/sysdeps/unix/sysv/linux/clock_nanosleep.c#L39

I propose to correct the vos_threadDelay() code as shown in the appended file. Please note that
nanosleep is completely suitable to do what is expected. I checked the function on a Linux aarch64 platform with glibc version 2.35, kernel version 5.10.168, gcc version 11.3.0

Regards
Walter

1 Attachments

Discussion

  • Armin-Hagen Weiss

    • Milestone: 2.1.0.0 --> 3.0.1.0
     
  • Armin-Hagen Weiss

    To avoid problems wit time jumps caused by time sync we have to use clock_nanosleep() for CLOCK_MONOTONIC. Thus we can unfortunately not use the proposed solution. Do You have a proposal for clock_nanosleep() ?

     
  • Walter Stoll

    Walter Stoll - 2024-02-06

    A attached a second proposal. This proposal takes vos_threadDealy() from version 3.0.0.0 with minimal adjustments. Please note, that with the TIMER_ABSTIME flag passed to clock_nanosleep(), the remaining time is not returned in remain. I refer herein to https://linux.die.net/man/2/clock_nanosleep

     
  • Armin-Hagen Weiss

    • labels: --> 2432
    • status: open --> closed
    • assigned_to: Armin-Hagen Weiss
     
  • Armin-Hagen Weiss

    Thank You Walter !

     
  • Armin-Hagen Weiss

    • labels: 2432 --> 2432, 2434
     

Log in to post a comment.