From 6872c14e1a02445d945623ee6e85230c5d7ecbce Mon Sep 17 00:00:00 2001 From: iphydf Date: Sun, 26 Aug 2018 20:34:26 +0000 Subject: Avoid use of global mutable state in mono_time on win32. This uses a trick to get read-write access to `this` from a `const` member function, similar to C++ `mutable`, but uglier. --- auto_tests/dht_test.c | 5 ++- auto_tests/run_auto_test.h | 5 ++- testing/av_test.c | 2 +- toxav/audio.c | 4 +- toxav/audio.h | 6 +-- toxav/rtp.h | 2 +- toxav/video.c | 4 +- toxav/video.h | 4 +- toxcore/mono_time.c | 97 +++++++++++++++++++++++----------------------- toxcore/mono_time.h | 4 +- toxcore/mono_time_test.cc | 4 +- toxcore/net_crypto.c | 2 +- 12 files changed, 71 insertions(+), 68 deletions(-) diff --git a/auto_tests/dht_test.c b/auto_tests/dht_test.c index 232b7d95..f9c09dd5 100644 --- a/auto_tests/dht_test.c +++ b/auto_tests/dht_test.c @@ -600,9 +600,10 @@ static void ip_callback(void *data, int32_t number, IP_Port ip_port) #define NUM_DHT_FRIENDS 20 -static uint64_t get_clock_callback(void *user_data) +static uint64_t get_clock_callback(Mono_Time *mono_time, void *user_data) { - return *(uint64_t *)user_data; + const uint64_t *clock = (const uint64_t *)user_data; + return *clock; } static void test_DHT_test(void) diff --git a/auto_tests/run_auto_test.h b/auto_tests/run_auto_test.h index 602ad524..bcbfed55 100644 --- a/auto_tests/run_auto_test.h +++ b/auto_tests/run_auto_test.h @@ -42,9 +42,10 @@ static void iterate_all_wait(uint32_t tox_count, Tox **toxes, State *state, uint c_sleep(20); } -static uint64_t get_state_clock_callback(void *user_data) +static uint64_t get_state_clock_callback(Mono_Time *mono_time, void *user_data) { - return ((State *)user_data)->clock; + const State *state = (const State *)user_data; + return state->clock; } static void run_auto_test(uint32_t tox_count, void test(Tox **toxes, State *state)) diff --git a/testing/av_test.c b/testing/av_test.c index 18bb3105..ca2b2779 100644 --- a/testing/av_test.c +++ b/testing/av_test.c @@ -569,7 +569,7 @@ CHECK_ARG: initialize_tox(&bootstrap, &AliceAV, &AliceCC, &BobAV, &BobCC); // TODO(iphydf): Don't depend on toxcore internals - const Mono_Time *mono_time = (*(Messenger **)bootstrap)->mono_time; + Mono_Time *mono_time = (*(Messenger **)bootstrap)->mono_time; if (TEST_TRANSFER_A) { SNDFILE *af_handle; diff --git a/toxav/audio.c b/toxav/audio.c index 5da41570..de007424 100644 --- a/toxav/audio.c +++ b/toxav/audio.c @@ -44,7 +44,7 @@ static bool reconfigure_audio_decoder(ACSession *ac, int32_t sampling_rate, int8 -ACSession *ac_new(const Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, +ACSession *ac_new(Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, toxav_audio_receive_frame_cb *cb, void *cb_data) { ACSession *ac = (ACSession *)calloc(sizeof(ACSession), 1); @@ -216,7 +216,7 @@ void ac_iterate(ACSession *ac) pthread_mutex_unlock(ac->queue_mutex); } -int ac_queue_message(const Mono_Time *mono_time, void *acp, struct RTPMessage *msg) +int ac_queue_message(Mono_Time *mono_time, void *acp, struct RTPMessage *msg) { if (!acp || !msg) { return -1; diff --git a/toxav/audio.h b/toxav/audio.h index 1a33a2ae..a323a08e 100644 --- a/toxav/audio.h +++ b/toxav/audio.h @@ -50,7 +50,7 @@ #define AUDIO_MAX_BUFFER_SIZE_BYTES (AUDIO_MAX_BUFFER_SIZE_PCM16 * 2) typedef struct ACSession_s { - const Mono_Time *mono_time; + Mono_Time *mono_time; const Logger *log; /* encoding */ @@ -78,11 +78,11 @@ typedef struct ACSession_s { void *acb_user_data; } ACSession; -ACSession *ac_new(const Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, +ACSession *ac_new(Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, toxav_audio_receive_frame_cb *cb, void *cb_data); void ac_kill(ACSession *ac); void ac_iterate(ACSession *ac); -int ac_queue_message(const Mono_Time *mono_time, void *acp, struct RTPMessage *msg); +int ac_queue_message(Mono_Time *mono_time, void *acp, struct RTPMessage *msg); int ac_reconfigure_encoder(ACSession *ac, int32_t bit_rate, int32_t sampling_rate, uint8_t channels); #endif /* AUDIO_H */ diff --git a/toxav/rtp.h b/toxav/rtp.h index 42997594..e068f6ac 100644 --- a/toxav/rtp.h +++ b/toxav/rtp.h @@ -159,7 +159,7 @@ struct RTPWorkBufferList { #define DISMISS_FIRST_LOST_VIDEO_PACKET_COUNT 10 -typedef int rtp_m_cb(const Mono_Time *mono_time, void *cs, struct RTPMessage *msg); +typedef int rtp_m_cb(Mono_Time *mono_time, void *cs, struct RTPMessage *msg); /** * RTP control session. diff --git a/toxav/video.c b/toxav/video.c index 3f4d96e1..ab47ae97 100644 --- a/toxav/video.c +++ b/toxav/video.c @@ -159,7 +159,7 @@ static void vc_init_encoder_cfg(const Logger *log, vpx_codec_enc_cfg_t *cfg, int #endif } -VCSession *vc_new(const Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, +VCSession *vc_new(Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, toxav_video_receive_frame_cb *cb, void *cb_data) { VCSession *vc = (VCSession *)calloc(sizeof(VCSession), 1); @@ -357,7 +357,7 @@ void vc_iterate(VCSession *vc) } } -int vc_queue_message(const Mono_Time *mono_time, void *vcp, struct RTPMessage *msg) +int vc_queue_message(Mono_Time *mono_time, void *vcp, struct RTPMessage *msg) { /* This function is called with complete messages * they have already been assembled. diff --git a/toxav/video.h b/toxav/video.h index 2eee1c05..16f4658b 100644 --- a/toxav/video.h +++ b/toxav/video.h @@ -60,11 +60,11 @@ typedef struct VCSession_s { pthread_mutex_t queue_mutex[1]; } VCSession; -VCSession *vc_new(const Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, +VCSession *vc_new(Mono_Time *mono_time, const Logger *log, ToxAV *av, uint32_t friend_number, toxav_video_receive_frame_cb *cb, void *cb_data); void vc_kill(VCSession *vc); void vc_iterate(VCSession *vc); -int vc_queue_message(const Mono_Time *mono_time, void *vcp, struct RTPMessage *msg); +int vc_queue_message(Mono_Time *mono_time, void *vcp, struct RTPMessage *msg); int vc_reconfigure_encoder(VCSession *vc, uint32_t bit_rate, uint16_t width, uint16_t height, int16_t kf_max_dist); #endif /* VIDEO_H */ diff --git a/toxcore/mono_time.c b/toxcore/mono_time.c index aa272749..6e4bc067 100644 --- a/toxcore/mono_time.c +++ b/toxcore/mono_time.c @@ -28,12 +28,52 @@ struct Mono_Time { uint64_t time; uint64_t base_time; +#ifdef OS_WIN32 + uint64_t last_clock_mono; + uint64_t add_clock_mono; +#endif mono_time_current_time_cb *current_time_callback; void *user_data; }; -static mono_time_current_time_cb current_time_monotonic_default; +static uint64_t current_time_monotonic_default(Mono_Time *mono_time, void *user_data) +{ + uint64_t time; +#ifdef OS_WIN32 + uint64_t old_add_clock_mono = mono_time->add_clock_mono; + time = (uint64_t)GetTickCount() + mono_time->add_clock_mono; + + /* Check if time has decreased because of 32 bit wrap from GetTickCount(), while avoiding false positives from race + * conditions when multiple threads call this function at once */ + if (time + 0x10000 < mono_time->last_clock_mono) { + uint32_t add = ~0; + /* use old_add_clock_mono rather than simply incrementing add_clock_mono, to handle the case that many threads + * simultaneously detect an overflow */ + mono_time->add_clock_mono = old_add_clock_mono + add; + time += add; + } + + mono_time->last_clock_mono = time; +#else + struct timespec clock_mono; +#if defined(__APPLE__) + clock_serv_t muhclock; + mach_timespec_t machtime; + + host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &muhclock); + clock_get_time(muhclock, &machtime); + mach_port_deallocate(mach_task_self(), muhclock); + + clock_mono.tv_sec = machtime.tv_sec; + clock_mono.tv_nsec = machtime.tv_nsec; +#else + clock_gettime(CLOCK_MONOTONIC, &clock_mono); +#endif + time = 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL); +#endif + return time; +} Mono_Time *mono_time_new(void) { @@ -46,6 +86,11 @@ Mono_Time *mono_time_new(void) mono_time->current_time_callback = current_time_monotonic_default; mono_time->user_data = nullptr; +#ifdef OS_WIN32 + mono_time->last_clock_mono = 0; + mono_time->add_clock_mono = 0; +#endif + mono_time->time = 0; mono_time->base_time = (uint64_t)time(nullptr) - (current_time_monotonic(mono_time) / 1000ULL); @@ -87,53 +132,7 @@ void mono_time_set_current_time_callback(Mono_Time *mono_time, } /* return current monotonic time in milliseconds (ms). */ -uint64_t current_time_monotonic(const Mono_Time *mono_time) +uint64_t current_time_monotonic(Mono_Time *mono_time) { - return mono_time->current_time_callback(mono_time->user_data); -} - -//!TOKSTYLE- -// No global mutable state in Tokstyle. -#ifdef OS_WIN32 -static uint64_t last_clock_mono; -static uint64_t add_clock_mono; -#endif -//!TOKSTYLE+ - -static uint64_t current_time_monotonic_default(void *user_data) -{ - uint64_t time; -#ifdef OS_WIN32 - uint64_t old_add_clock_mono = add_clock_mono; - time = (uint64_t)GetTickCount() + add_clock_mono; - - /* Check if time has decreased because of 32 bit wrap from GetTickCount(), while avoiding false positives from race - * conditions when multiple threads call this function at once */ - if (time + 0x10000 < last_clock_mono) { - uint32_t add = ~0; - /* use old_add_clock_mono rather than simply incrementing add_clock_mono, to handle the case that many threads - * simultaneously detect an overflow */ - add_clock_mono = old_add_clock_mono + add; - time += add; - } - - last_clock_mono = time; -#else - struct timespec clock_mono; -#if defined(__APPLE__) - clock_serv_t muhclock; - mach_timespec_t machtime; - - host_get_clock_service(mach_host_self(), SYSTEM_CLOCK, &muhclock); - clock_get_time(muhclock, &machtime); - mach_port_deallocate(mach_task_self(), muhclock); - - clock_mono.tv_sec = machtime.tv_sec; - clock_mono.tv_nsec = machtime.tv_nsec; -#else - clock_gettime(CLOCK_MONOTONIC, &clock_mono); -#endif - time = 1000ULL * clock_mono.tv_sec + (clock_mono.tv_nsec / 1000000ULL); -#endif - return time; + return mono_time->current_time_callback(mono_time, mono_time->user_data); } diff --git a/toxcore/mono_time.h b/toxcore/mono_time.h index b735f2c3..c2934d06 100644 --- a/toxcore/mono_time.h +++ b/toxcore/mono_time.h @@ -50,9 +50,9 @@ uint64_t mono_time_get(const Mono_Time *mono_time); bool mono_time_is_timeout(const Mono_Time *mono_time, uint64_t timestamp, uint64_t timeout); /* return current monotonic time in milliseconds (ms). */ -uint64_t current_time_monotonic(const Mono_Time *mono_time); +uint64_t current_time_monotonic(Mono_Time *mono_time); -typedef uint64_t mono_time_current_time_cb(void *user_data); +typedef uint64_t mono_time_current_time_cb(Mono_Time *mono_time, void *user_data); /* Override implementation of current_time_monotonic() (for tests). * diff --git a/toxcore/mono_time_test.cc b/toxcore/mono_time_test.cc index 6e81ac7e..3b33a241 100644 --- a/toxcore/mono_time_test.cc +++ b/toxcore/mono_time_test.cc @@ -35,7 +35,9 @@ TEST(MonoTime, IsTimeout) { mono_time_free(mono_time); } -static uint64_t test_current_time_callback(void *user_data) { return *(uint64_t *)user_data; } +uint64_t test_current_time_callback(Mono_Time *mono_time, void *user_data) { + return *(uint64_t *)user_data; +} TEST(MonoTime, CustomTime) { Mono_Time *mono_time = mono_time_new(); diff --git a/toxcore/net_crypto.c b/toxcore/net_crypto.c index a64ed970..736b9c4a 100644 --- a/toxcore/net_crypto.c +++ b/toxcore/net_crypto.c @@ -972,7 +972,7 @@ static int generate_request_packet(const Logger *log, uint8_t *data, uint16_t le * return -1 on failure. * return number of requested packets on success. */ -static int handle_request_packet(const Mono_Time *mono_time, const Logger *log, Packets_Array *send_array, +static int handle_request_packet(Mono_Time *mono_time, const Logger *log, Packets_Array *send_array, const uint8_t *data, uint16_t length, uint64_t *latest_send_time, uint64_t rtt_time) { if (length == 0) { -- cgit v1.2.3