Skip to content

Commit

Permalink
Prevents OS::get_ticks_usec() from going backwards in time due to OS …
Browse files Browse the repository at this point in the history
…bugs.

On some hardware / OSes calls to timing APIs may occasionally falsely give the impression time is running backwards (e.g. QueryPerformanceCounter). This can cause bugs in any Godot code that assumes time always goes forwards. This PR simply records the previous time returned by the OS, and returns the previous time if the new time is earlier than the previous time.

May fix #31837, #25166, #27887.
  • Loading branch information
lawnjelly committed Sep 2, 2019
1 parent 979e772 commit 8293222
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 12 deletions.
57 changes: 56 additions & 1 deletion core/os/os.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ OS *OS::get_singleton() {
return singleton;
}

uint32_t OS::get_ticks_msec() const {
uint32_t OS::get_ticks_msec() {
return get_ticks_usec() / 1000;
}

Expand Down Expand Up @@ -93,6 +93,59 @@ uint64_t OS::get_system_time_secs() const {
uint64_t OS::get_system_time_msecs() const {
return 0;
}

uint64_t OS::get_ticks_usec() {
// This is a workaround for rare OS bugs where a call to get_ticks_raw_usec()
// may return an EARLIER value than that given in a previous call.
// This ensures that the minimum delta will be zero, and prevents bugs which rely on
// the delta being positive.
uint64_t t = get_ticks_raw_usec();

// note that get_ticks_raw_usec() may return 0 if the OS API function failed.
// we will deal with this by returning the previous value
if (t == 0)
return _ticks_usec_running_total;

if (t >= _ticks_usec_prev)
{
// time going forwards, normal case
uint64_t diff = t - _ticks_usec_prev;

// cap forward time (shouldn't happen except for long stalls, but just in case)
if (diff > 100000)
{
if (diff < 1000000) {
WARN_PRINTS("OS time running forward " + itos(diff) + " usecs detected");
}
else {
WARN_PRINTS("OS time running forward " + itos(diff / 1000000) + " secs detected");
}
diff = 100000;
}

// move our own robust 'fake clock' forward
_ticks_usec_running_total += diff;
}
else
{
// time going backwards, we need to deal with this
// Do not increment running total
uint64_t diff = _ticks_usec_prev - t;

if (diff < 1000000) {
WARN_PRINTS("OS time running backward " + itos(diff) + " usecs detected");
}
else {
WARN_PRINTS("OS time running backward " + itos(diff / 1000000) + " secs detected");
}
}

// resync each time, the difference should be based on the previous value
_ticks_usec_prev = t;

return _ticks_usec_running_total;
}

void OS::debug_break(){

// something
Expand Down Expand Up @@ -769,6 +822,8 @@ OS::OS() {
_keep_screen_on = true; // set default value to true, because this had been true before godot 2.0.
low_processor_usage_mode = false;
low_processor_usage_mode_sleep_usec = 10000;
_ticks_usec_prev = 0;
_ticks_usec_running_total = 0;
_verbose_stdout = false;
_no_window = false;
_exit_code = 0;
Expand Down
8 changes: 6 additions & 2 deletions core/os/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class OS {
bool _allow_hidpi;
bool _allow_layered;
bool _use_vsync;
uint64_t _ticks_usec_prev;
uint64_t _ticks_usec_running_total;

char *last_error;

Expand Down Expand Up @@ -138,6 +140,8 @@ class OS {
void _ensure_user_data_dir();
virtual bool _check_internal_feature_support(const String &p_feature) = 0;

virtual uint64_t get_ticks_raw_usec() const = 0;

public:
typedef int64_t ProcessID;

Expand Down Expand Up @@ -344,8 +348,8 @@ class OS {
virtual uint64_t get_system_time_msecs() const;

virtual void delay_usec(uint32_t p_usec) const = 0;
virtual uint64_t get_ticks_usec() const = 0;
uint32_t get_ticks_msec() const;
uint64_t get_ticks_usec();
uint32_t get_ticks_msec();
uint64_t get_splash_tick_msec() const;

virtual bool can_draw() const = 0;
Expand Down
21 changes: 17 additions & 4 deletions drivers/unix/os_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,17 +258,30 @@ void OS_Unix::delay_usec(uint32_t p_usec) const {
while (nanosleep(&rem, &rem) == EINTR) {
}
}
uint64_t OS_Unix::get_ticks_usec() const {
uint64_t OS_Unix::get_ticks_raw_usec() const {

#if defined(__APPLE__)
uint64_t longtime = mach_absolute_time() * _clock_scale;
#else
// Unchecked return. Static analyzers might complain.
// If _setup_clock() succeeded, we assume clock_gettime() works.
struct timespec tv_now = { 0, 0 };
clock_gettime(GODOT_CLOCK, &tv_now);

int res = clock_gettime(GODOT_CLOCK, &tv_now);

// if clock_gettime failed, return zero which is dealt with by wrapper
if (res != 0)
{
WARN_PRINT_ONCE("clock_gettime failed");
return 0;
}

uint64_t longtime = ((uint64_t)tv_now.tv_nsec / 1000L) + (uint64_t)tv_now.tv_sec * 1000000L;
#endif
// _clock_start should never be ahead of longtime
if (_clock_start > longtime) {
WARN_PRINT_ONCE("_clock_start is ahead of clock_gettime");
return 0;
}

longtime -= _clock_start;

return longtime;
Expand Down
3 changes: 2 additions & 1 deletion drivers/unix/os_unix.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class OS_Unix : public OS {

virtual void finalize_core();

virtual uint64_t get_ticks_raw_usec() const;

String stdin_buf;

public:
Expand Down Expand Up @@ -83,7 +85,6 @@ class OS_Unix : public OS {
virtual uint64_t get_system_time_msecs() const;

virtual void delay_usec(uint32_t p_usec) const;
virtual uint64_t get_ticks_usec() const;

virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id = NULL, String *r_pipe = NULL, int *r_exitcode = NULL, bool read_stderr = false, Mutex *p_pipe_mutex = NULL);
virtual Error kill(const ProcessID &p_pid);
Expand Down
2 changes: 1 addition & 1 deletion platform/uwp/os_uwp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ void OS_UWP::delay_usec(uint32_t p_usec) const {
// no Sleep()
WaitForSingleObjectEx(GetCurrentThread(), msec, false);
}
uint64_t OS_UWP::get_ticks_usec() const {
uint64_t OS_UWP::get_ticks_raw_usec() const {

uint64_t ticks;
uint64_t time;
Expand Down
3 changes: 2 additions & 1 deletion platform/uwp/os_uwp.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ class OS_UWP : public OS {

void process_key_events();

virtual uint64_t get_ticks_raw_usec() const;

public:
// Event to send to the app wrapper
HANDLE mouse_mode_changed;
Expand Down Expand Up @@ -206,7 +208,6 @@ class OS_UWP : public OS {
virtual Error set_cwd(const String &p_cwd);

virtual void delay_usec(uint32_t p_usec) const;
virtual uint64_t get_ticks_usec() const;

virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id = NULL, String *r_pipe = NULL, int *r_exitcode = NULL, bool read_stderr = false, Mutex *p_pipe_mutex = NULL);
virtual Error kill(const ProcessID &p_pid);
Expand Down
2 changes: 1 addition & 1 deletion platform/windows/os_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2284,7 +2284,7 @@ void OS_Windows::delay_usec(uint32_t p_usec) const {
else
Sleep(p_usec / 1000);
}
uint64_t OS_Windows::get_ticks_usec() const {
uint64_t OS_Windows::get_ticks_raw_usec() const {

uint64_t ticks;
uint64_t time;
Expand Down
3 changes: 2 additions & 1 deletion platform/windows/os_windows.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ class OS_Windows : public OS {
void process_events();
void process_key_events();

virtual uint64_t get_ticks_raw_usec() const;

struct ProcessInfo {

STARTUPINFO si;
Expand Down Expand Up @@ -287,7 +289,6 @@ class OS_Windows : public OS {
virtual Error set_cwd(const String &p_cwd);

virtual void delay_usec(uint32_t p_usec) const;
virtual uint64_t get_ticks_usec() const;

virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id = NULL, String *r_pipe = NULL, int *r_exitcode = NULL, bool read_stderr = false, Mutex *p_pipe_mutex = NULL);
virtual Error kill(const ProcessID &p_pid);
Expand Down

0 comments on commit 8293222

Please sign in to comment.