wolfcw / libfaketime

libfaketime modifies the system time for a single application
https://github.com/wolfcw/libfaketime
GNU General Public License v2.0
2.62k stars 319 forks source link

FAKETIME_SPAWN_SECONDS should not be tested for equality #424

Closed kirelagin closed 1 year ago

kirelagin commented 1 year ago

The docs could be more explicit, but, still, I think it is pretty clear:

(... usual libfaketime setup here, setting LD_PRELOAD and FAKETIME ...)
export FAKETIME_SPAWN_TARGET="/bin/echo 'Hello world'"
export FAKETIME_SPAWN_SECONDS=5
/opt/local/bin/myprogram

This will run the "echo" command with the given parameter during the first
time-related system function call that "myprogram" performs after running for 5
seconds.

My reading of:

during the first time-related system function call <...> after running for 5 seconds

is that after the program runs for 5 seconds, the first time function call, whenever it happens (e.g. after 10 seconds) will trigger the external process. I also think this is the only interpretation that makes sense, given that, in general, it is hard to predict how the execution of a program goes, e.g. it can be preempted for a long time or whatever.

However, the current code will only spawn the process if the function call happens exactly after 5 seconds (to be more precise, any time during the fifth and only fifth second of the execution).


Reproduction:

#include <stdio.h>
#include <time.h>
#include <unistd.h>

int main() {
  printf("Going to sleep... time = %lld\n", (long long)time(0));
  sleep(2);
  printf("Woke up! time = %lld\n", (long long)time(0));

  return 0;
}

Expected output:

$ LD_PRELOAD=<...>/libfaketimeMT.so.1 FAKETIME="@1970-01-01 00:00:20" FAKETIME_SPAWN_SECONDS=1 FAKETIME_SPAWN_TARGET="echo hi" ./test
Going to sleep... time = 18020
hi
Woke up! time = 18022

Actual output:

$ LD_PRELOAD=<...>/libfaketimeMT.so.1 FAKETIME="@1970-01-01 00:00:20" FAKETIME_SPAWN_SECONDS=1 FAKETIME_SPAWN_TARGET="echo hi" ./test
Going to sleep... time = 18020
Woke up! time = 18022
wolfcw commented 1 year ago

Yes, agreed. :-)

Would you want to submit a PR changing the first == into >= in that line? Apparently noone noticed for 10 years, so you deserve all the credit. :-)

kirelagin commented 1 year ago

Credit goes not to me but to another person (who left the company long before I started working here) who fixed this 9 years ago. I only took the time to report the issue 🙂.

It is complicated, so it will be much easier if you just fix it, rather than me going through legal and compliance for one character. (Actually, it is not one character because you'll want to make sure that ft_spawn_secs is not -1 and also it might or might not make sense to apply the same transformation to ft_spawn_ncalls (I am not sure if it makes sense for this one)).