gh-125997: ensure that time.sleep(0) is not delayed on non-Windows platforms

[picnixz]

This is a suggestion for fixing the issue when the timeout is 0. For other timeouts, this does not change the current behaviour (I'm tempted removing the do-while loop as we should never have an issue, but I'm not familar with the monotonic C implementation so I left it as is).

• Issue: time.sleep(0) regression in 3.11 onwards #125997

[rruuaanng]

@picnixz Unfortunately, we should seriously review :(. It seems that an assertion failed somewhere.

Assertion failed: (errno == 0), function pysleep, file timemodule.c, line 2220.
Fatal Python error: Aborted

Thread 0x0000000173bbb000 (most recent call first):
  File "/Users/admin/actions-runner/_work/cpython/cpython/Lib/subprocess.py", line 1918 in _execute_child
...

[picnixz]

Ah I think I know what happens. When we raise an OSError from errno we don't clear the errno. So if you chain calls, you need to just ignore them. I'm removing the assertion cc @ZeroIntensity

[rruuaanng]

If pytime is running on multiple cores, I recommend using _LIBC_REENTRANT, which will make your errno independent in each thread.

raw defined

#  if !defined _LIBC || defined _LIBC_REENTRANT
/* When using threads, errno is a per-thread value.  */
#   define errno (*__errno_location ())
#  endif
# endif /* !__ASSEMBLER__ */
#endif /* _ERRNO_H */

[picnixz]

This wouldn't solve the issue (also because _LIBC_REENTRANT is a private macro and something you supply when configuring libc). By the way, errno is thread-safe (see https://linux.die.net/man/3/errno). The issue is simply that we can raise an OSError from errno, which doesn't clear errno, and do something else afterwards. I will keep the current behaviour as it was done beforehand.

[ZeroIntensity]

TIL! Should we assert !PyErr_Occurred() instead?

[picnixz]

TIL! Should we assert !PyErr_Occurred() instead?

Not sure. It's not really an issue and the function is internal and only for time.sleep(). It's only used once (it's just that time.sleep() calls this function). Though it wouldn't hurt (if this assertion fails, then there is a separate issue)

[ZeroIntensity]

I think we should. There's no real cost to adding it, and it would be helpful for debugging in the rare case that something goes wrong.