--exclude-monotonic broken with clock_nanosleep() and TIMER_ABSTIME

[vcunat]

Simple test case:

$ echo -e "import time\ntime.sleep(1)" | faketime --exclude-monotonic -f +1h python3.11
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
OSError: [Errno 22] Invalid argument

What happens is that python calls

clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME, &timeout_abs, NULL);

and with this setting faketime fumbles, passes a negative time to the kernel syscall, which is how EINVAL happens.

[vcunat]

This behavior of python only starts with 3.11, but the bug really seems to be in faketime.

[vcunat]

I don't know faketime codebase, but I'd expect that when --exclude-monotonic is active, it wouldn't touch the timestamp, so e.g.

--- a/src/libfaketime.c
+++ b/src/libfaketime.c
@@ -1359 +1359 @@ int clock_nanosleep(clockid_t clock_id, int flags, const struct timespec *req, s
-      else if (clock_id == CLOCK_MONOTONIC)
+      else if (clock_id == CLOCK_MONOTONIC && fake_monotonic_clock)

or perhaps with some details changed, like re-reading from env vars, e.g. (my ugly code)

--- a/src/libfaketime.c
+++ b/src/libfaketime.c
@@ -1359 +1359,2 @@ int clock_nanosleep(clockid_t clock_id, int flags, const struct timespec *req, s
-      else if (clock_id == CLOCK_MONOTONIC)
+      else if (clock_id == CLOCK_MONOTONIC &&
+                (get_fake_monotonic_setting(&fake_monotonic_clock), fake_monotonic_clock))

This diff fixes the trivial test case above, at least.

[Spiffyk]

Commit d17bb11 does not fix the issue for me - the OSError remains unchanged. @vcunat's second patch with the get_fake_monotonic_setting call seems to do the trick.

[wolfcw]

hm, somewhat weird, since this should be handled by initialisation even if clock_nanotime() is the first call. Added the explicit update as outlined be @vcunat ... could you give it another spin?

[vcunat]

Sounds weird. I believe that in our use case we trigger it only like faketime --exclude-monotonic, not by setting during runtime. EDIT: but I haven't verified faketime's startup code in any way.

[vcunat]

Wait... isn't the problem that you need to do else { real_req = *req; } which your change does not? (contrary to mine)

[vcunat]

I mean, I haven't studied the code in detail, but the else-part was exactly why I was writing it in such an ugly condition.

[wolfcw]

you're absolutely right

[vcunat]

So this get_* is probably not needed, but it shouldn't matter. I don't expect it would be a noticeable overhead.

[Spiffyk]

I can confirm the latest commit works

[vcunat]

Looks good to me now.