diff options
author | sudden6 <sudden6@gmx.at> | 2019-09-06 15:12:20 +0200 |
---|---|---|
committer | sudden6 <sudden6@gmx.at> | 2019-09-06 23:10:39 +0200 |
commit | 571897cecbdc96bc48e2778e06b9102bdca4839a (patch) | |
tree | 2dd4b11a43ef8963671a7e01783b1c474f43d993 | |
parent | 3d21e66a8a7bf06507f8e62a5cc78f89f86adbc7 (diff) |
fix concurrency issues in mono_time
Since mono_time is accessed from the main thread as well as the toxav
thread it is needed to properly lock time updates.
-rw-r--r-- | toxcore/BUILD.bazel | 5 | ||||
-rw-r--r-- | toxcore/mono_time.c | 71 |
2 files changed, 57 insertions, 19 deletions
diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index b21ec4a6..fc28a333 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel | |||
@@ -63,7 +63,10 @@ cc_library( | |||
63 | name = "mono_time", | 63 | name = "mono_time", |
64 | srcs = ["mono_time.c"], | 64 | srcs = ["mono_time.c"], |
65 | hdrs = ["mono_time.h"], | 65 | hdrs = ["mono_time.h"], |
66 | deps = [":ccompat"], | 66 | deps = [ |
67 | ":ccompat", | ||
68 | "@pthread", | ||
69 | ], | ||
67 | ) | 70 | ) |
68 | 71 | ||
69 | cc_test( | 72 | cc_test( |
diff --git a/toxcore/mono_time.c b/toxcore/mono_time.c index 6e4bc067..a8dae350 100644 --- a/toxcore/mono_time.c +++ b/toxcore/mono_time.c | |||
@@ -19,6 +19,7 @@ | |||
19 | 19 | ||
20 | #include "mono_time.h" | 20 | #include "mono_time.h" |
21 | 21 | ||
22 | #include <pthread.h> | ||
22 | #include <stdlib.h> | 23 | #include <stdlib.h> |
23 | #include <time.h> | 24 | #include <time.h> |
24 | 25 | ||
@@ -29,32 +30,44 @@ struct Mono_Time { | |||
29 | uint64_t time; | 30 | uint64_t time; |
30 | uint64_t base_time; | 31 | uint64_t base_time; |
31 | #ifdef OS_WIN32 | 32 | #ifdef OS_WIN32 |
32 | uint64_t last_clock_mono; | 33 | uint32_t last_clock_mono; |
33 | uint64_t add_clock_mono; | ||
34 | #endif | 34 | #endif |
35 | 35 | ||
36 | /* protect `time` and `last_clock_mono` from concurrent access */ | ||
37 | pthread_rwlock_t *time_update_lock; | ||
38 | |||
36 | mono_time_current_time_cb *current_time_callback; | 39 | mono_time_current_time_cb *current_time_callback; |
37 | void *user_data; | 40 | void *user_data; |
38 | }; | 41 | }; |
39 | 42 | ||
40 | static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) | 43 | static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) |
41 | { | 44 | { |
42 | uint64_t time; | 45 | uint64_t time = 0; |
43 | #ifdef OS_WIN32 | 46 | #ifdef OS_WIN32 |
44 | uint64_t old_add_clock_mono = mono_time->add_clock_mono; | 47 | /* let only one thread at a time check for overflow */ |
45 | time = (uint64_t)GetTickCount() + mono_time->add_clock_mono; | 48 | pthread_rwlock_wrlock(mono_time->time_update_lock); |
46 | 49 | ||
47 | /* Check if time has decreased because of 32 bit wrap from GetTickCount(), while avoiding false positives from race | 50 | /* GetTickCount provides only a 32 bit counter, but we can't use |
48 | * conditions when multiple threads call this function at once */ | 51 | * GetTickCount64 for backwards compatibility, so we handle wraparound |
49 | if (time + 0x10000 < mono_time->last_clock_mono) { | 52 | * ourselfes. |
50 | uint32_t add = ~0; | 53 | */ |
51 | /* use old_add_clock_mono rather than simply incrementing add_clock_mono, to handle the case that many threads | 54 | uint32_t ticks = GetTickCount(); |
52 | * simultaneously detect an overflow */ | 55 | |
53 | mono_time->add_clock_mono = old_add_clock_mono + add; | 56 | /* the higher 32 bits count the amount of overflows */ |
54 | time += add; | 57 | uint64_t old_ovf = mono_time->time & ~((uint64_t)UINT32_MAX); |
58 | |||
59 | /* Check if time has decreased because of 32 bit wrap from GetTickCount() */ | ||
60 | if (ticks < mono_time->last_clock_mono) { | ||
61 | /* account for overflow */ | ||
62 | old_ovf += UINT32_MAX + UINT64_C(1); | ||
55 | } | 63 | } |
56 | 64 | ||
57 | mono_time->last_clock_mono = time; | 65 | mono_time->last_clock_mono = ticks; |
66 | |||
67 | /* splice the low and high bits back together */ | ||
68 | time = old_ovf + ticks; | ||
69 | |||
70 | pthread_rwlock_unlock(mono_time->time_update_lock); | ||
58 | #else | 71 | #else |
59 | struct timespec clock_mono; | 72 | struct timespec clock_mono; |
60 | #if defined(__APPLE__) | 73 | #if defined(__APPLE__) |
@@ -83,17 +96,30 @@ Mono_Time *mono_time_new(void) | |||
83 | return nullptr; | 96 | return nullptr; |
84 | } | 97 | } |
85 | 98 | ||
99 | mono_time->time_update_lock = (pthread_rwlock_t *)malloc(sizeof(pthread_rwlock_t)); | ||
100 | |||
101 | if (mono_time->time_update_lock == nullptr) { | ||
102 | free(mono_time); | ||
103 | return nullptr; | ||
104 | } | ||
105 | |||
106 | if (pthread_rwlock_init(mono_time->time_update_lock, nullptr) < 0) { | ||
107 | free(mono_time->time_update_lock); | ||
108 | free(mono_time); | ||
109 | return nullptr; | ||
110 | } | ||
111 | |||
86 | mono_time->current_time_callback = current_time_monotonic_default; | 112 | mono_time->current_time_callback = current_time_monotonic_default; |
87 | mono_time->user_data = nullptr; | 113 | mono_time->user_data = nullptr; |
88 | 114 | ||
89 | #ifdef OS_WIN32 | 115 | #ifdef OS_WIN32 |
90 | mono_time->last_clock_mono = 0; | 116 | mono_time->last_clock_mono = 0; |
91 | mono_time->add_clock_mono = 0; | ||
92 | #endif | 117 | #endif |
93 | 118 | ||
94 | mono_time->time = 0; | 119 | mono_time->time = 0; |
95 | mono_time->base_time = (uint64_t)time(nullptr) - (current_time_monotonic(mono_time) / 1000ULL); | 120 | mono_time->base_time = (uint64_t)time(nullptr) - (current_time_monotonic(mono_time) / 1000ULL); |
96 | 121 | ||
122 | |||
97 | mono_time_update(mono_time); | 123 | mono_time_update(mono_time); |
98 | 124 | ||
99 | return mono_time; | 125 | return mono_time; |
@@ -101,17 +127,26 @@ Mono_Time *mono_time_new(void) | |||
101 | 127 | ||
102 | void mono_time_free(Mono_Time *mono_time) | 128 | void mono_time_free(Mono_Time *mono_time) |
103 | { | 129 | { |
130 | pthread_rwlock_destroy(mono_time->time_update_lock); | ||
131 | free(mono_time->time_update_lock); | ||
104 | free(mono_time); | 132 | free(mono_time); |
105 | } | 133 | } |
106 | 134 | ||
107 | void mono_time_update(Mono_Time *mono_time) | 135 | void mono_time_update(Mono_Time *mono_time) |
108 | { | 136 | { |
109 | mono_time->time = (current_time_monotonic(mono_time) / 1000ULL) + mono_time->base_time; | 137 | uint64_t time = (current_time_monotonic(mono_time) / 1000ULL) + mono_time->base_time; |
138 | pthread_rwlock_wrlock(mono_time->time_update_lock); | ||
139 | mono_time->time = time; | ||
140 | pthread_rwlock_unlock(mono_time->time_update_lock); | ||
110 | } | 141 | } |
111 | 142 | ||
112 | uint64_t mono_time_get(const Mono_Time *mono_time) | 143 | uint64_t mono_time_get(const Mono_Time *mono_time) |
113 | { | 144 | { |
114 | return mono_time->time; | 145 | uint64_t time = 0; |
146 | pthread_rwlock_rdlock(mono_time->time_update_lock); | ||
147 | time = mono_time->time; | ||
148 | pthread_rwlock_unlock(mono_time->time_update_lock); | ||
149 | return time; | ||
115 | } | 150 | } |
116 | 151 | ||
117 | bool mono_time_is_timeout(const Mono_Time *mono_time, uint64_t timestamp, uint64_t timeout) | 152 | bool mono_time_is_timeout(const Mono_Time *mono_time, uint64_t timestamp, uint64_t timeout) |