From 75c83fed96dde63c1d372a9b045afd6b79ceff6d Mon Sep 17 00:00:00 2001 From: "William E. Kempf" Date: Thu, 15 Aug 2002 00:05:54 +0000 Subject: [PATCH] Fixed time precision bugs. Switched to Interlocked* methods for once. [SVN r14867] --- include/boost/thread/once.hpp | 4 ++-- src/condition.cpp | 26 +++++++++++++++++----- src/mutex.cpp | 23 ++++++++++++++----- src/once.cpp | 17 +++----------- src/recursive_mutex.cpp | 42 ++++++++++++++++++++++------------- src/thread.cpp | 8 +++---- src/timeconv.inl | 6 ++--- test/test_condition.cpp | 25 +++++++++++++-------- test/test_mutex.cpp | 18 +++++++++++++++ test/test_thread.cpp | 4 ++-- test/test_xtime.cpp | 18 ++++++++++++++- 11 files changed, 130 insertions(+), 61 deletions(-) diff --git a/include/boost/thread/once.hpp b/include/boost/thread/once.hpp index 43835422..05449f22 100644 --- a/include/boost/thread/once.hpp +++ b/include/boost/thread/once.hpp @@ -30,8 +30,8 @@ typedef pthread_once_t once_flag; #elif (defined(BOOST_HAS_WINTHREADS) || defined(BOOST_HAS_MPTASKS)) -typedef bool once_flag; -#define BOOST_ONCE_INIT false +typedef long once_flag; +#define BOOST_ONCE_INIT 0 #endif diff --git a/src/condition.cpp b/src/condition.cpp index eec2c145..0f5948a1 100644 --- a/src/condition.cpp +++ b/src/condition.cpp @@ -244,14 +244,28 @@ void condition::do_wait() bool condition::do_timed_wait(const xtime& xt) { - int milliseconds; - to_duration(xt, milliseconds); - + bool ret = false; unsigned int res = 0; - res = WaitForSingleObject(reinterpret_cast(m_queue), milliseconds); - assert(res != WAIT_FAILED && res != WAIT_ABANDONED); - bool ret = (res == WAIT_OBJECT_0); + for (;;) + { + int milliseconds; + to_duration(xt, milliseconds); + + res = WaitForSingleObject(reinterpret_cast(m_queue), milliseconds); + assert(res != WAIT_FAILED && res != WAIT_ABANDONED); + ret = (res == WAIT_OBJECT_0); + + if (res == WAIT_TIMEOUT) + { + xtime cur; + xtime_get(&cur, TIME_UTC); + if (xtime_cmp(xt, cur) > 0) + continue; + } + + break; + } unsigned was_waiting=0; unsigned was_gone=0; diff --git a/src/mutex.cpp b/src/mutex.cpp index 4999a84c..46ac92aa 100644 --- a/src/mutex.cpp +++ b/src/mutex.cpp @@ -145,12 +145,25 @@ bool timed_mutex::do_trylock() bool timed_mutex::do_timedlock(const xtime& xt) { - int milliseconds; - to_duration(xt, milliseconds); + unsigned int res = 0; + for (;;) + { + int milliseconds; + to_duration(xt, milliseconds); - unsigned int res = WaitForSingleObject(reinterpret_cast(m_mutex), milliseconds); - assert(res != WAIT_FAILED && res != WAIT_ABANDONED); - return res == WAIT_OBJECT_0; + res = WaitForSingleObject(reinterpret_cast(m_mutex), milliseconds); + assert(res != WAIT_FAILED && res != WAIT_ABANDONED); + + if (res == WAIT_TIMEOUT) + { + xtime cur; + xtime_get(&cur, TIME_UTC); + if (xtime_cmp(xt, cur) > 0) + continue; + } + + return res == WAIT_OBJECT_0; + } } void timed_mutex::do_unlock() diff --git a/src/once.cpp b/src/once.cpp index 9139fa56..82e4b5de 100644 --- a/src/once.cpp +++ b/src/once.cpp @@ -78,12 +78,7 @@ namespace boost { void call_once(void (*func)(), once_flag& flag) { #if defined(BOOST_HAS_WINTHREADS) - once_flag tmp = flag; - - // Memory barrier would be needed here to prevent race conditions on some platforms with - // partial ordering. - - if (!tmp) + if (InterlockedCompareExchange(&flag, 1, 1) == 0) { #if defined(BOOST_NO_STRINGSTREAM) std::ostrstream strm; @@ -101,16 +96,10 @@ void call_once(void (*func)(), once_flag& flag) res = WaitForSingleObject(mutex, INFINITE); assert(res == WAIT_OBJECT_0); - tmp = flag; - if (!tmp) + if (InterlockedCompareExchange(&flag, 1, 1) == 0) { func(); - tmp = true; - - // Memory barrier would be needed here to prevent race conditions on some platforms - // with partial ordering. - - flag = tmp; + InterlockedExchange(&flag, 1); } res = ReleaseMutex(mutex); diff --git a/src/recursive_mutex.cpp b/src/recursive_mutex.cpp index 0041b771..851c09a8 100644 --- a/src/recursive_mutex.cpp +++ b/src/recursive_mutex.cpp @@ -195,23 +195,35 @@ bool recursive_timed_mutex::do_trylock() bool recursive_timed_mutex::do_timedlock(const xtime& xt) { - int milliseconds; - to_duration(xt, milliseconds); + for (;;) + { + int milliseconds; + to_duration(xt, milliseconds); - unsigned int res = 0; - res = WaitForSingleObject(reinterpret_cast(m_mutex), milliseconds); - assert(res != WAIT_FAILED && res != WAIT_ABANDONED); + unsigned int res = 0; + res = WaitForSingleObject(reinterpret_cast(m_mutex), milliseconds); + assert(res != WAIT_FAILED && res != WAIT_ABANDONED); - if (res == WAIT_OBJECT_0) - { - if (++m_count > 1) - { - res = ReleaseMutex(reinterpret_cast(m_mutex)); - assert(res); - } - return true; - } - return false; + if (res == WAIT_TIMEOUT) + { + xtime cur; + xtime_get(&cur, TIME_UTC); + if (xtime_cmp(xt, cur) > 0) + continue; + } + + if (res == WAIT_OBJECT_0) + { + if (++m_count > 1) + { + res = ReleaseMutex(reinterpret_cast(m_mutex)); + assert(res); + } + return true; + } + + return false; + } } void recursive_timed_mutex::do_unlock() diff --git a/src/thread.cpp b/src/thread.cpp index 4a8b8654..cd39f553 100644 --- a/src/thread.cpp +++ b/src/thread.cpp @@ -191,14 +191,12 @@ void thread::join() void thread::sleep(const xtime& xt) { - for (;;) + for (int foo=0; foo < 5; ++foo) { #if defined(BOOST_HAS_WINTHREADS) int milliseconds; to_duration(xt, milliseconds); Sleep(milliseconds); - xtime xt2; - xtime_get(&xt2, TIME_UTC); #elif defined(BOOST_HAS_PTHREADS) # if defined(BOOST_HAS_PTHREAD_DELAY_NP) timespec ts; @@ -226,7 +224,9 @@ void thread::sleep(const xtime& xt) AbsoluteTime sWakeTime(DurationToAbsolute(lMicroseconds)); threads::mac::detail::safe_delay_until(&sWakeTime); #endif - if (xtime_cmp(xt, xt2) >= 0) + xtime cur; + xtime_get(&cur, TIME_UTC); + if (xtime_cmp(xt, cur) <= 0) return; } } diff --git a/src/timeconv.inl b/src/timeconv.inl index a82cbb88..e2d6c403 100644 --- a/src/timeconv.inl +++ b/src/timeconv.inl @@ -59,7 +59,7 @@ namespace { res = boost::xtime_get(&cur, boost::TIME_UTC); assert(res == boost::TIME_UTC); - if (xt.sec < cur.sec || (xt.sec == cur.sec && xt.nsec < cur.nsec)) + if (boost::xtime_cmp(xt, cur) <= 0) { ts.tv_sec = 0; ts.tv_nsec = 0; @@ -90,7 +90,7 @@ namespace { res = boost::xtime_get(&cur, boost::TIME_UTC); assert(res == boost::TIME_UTC); - if (xt.sec < cur.sec || (xt.sec == cur.sec && xt.nsec < cur.nsec)) + if (boost::xtime_cmp(xt, cur) <= 0) milliseconds = 0; else { @@ -107,7 +107,7 @@ namespace { res = boost::xtime_get(&cur, boost::TIME_UTC); assert(res == boost::TIME_UTC); - if (xt.sec < cur.sec || (xt.sec == cur.sec && xt.nsec < cur.nsec)) + if (boost::xtime_get(&cur, boost::TIME_UTC) <= 0) microseconds = 0; else { diff --git a/test/test_condition.cpp b/test/test_condition.cpp index 548e610a..9a94b1e2 100644 --- a/test/test_condition.cpp +++ b/test/test_condition.cpp @@ -72,7 +72,7 @@ namespace // Test timed_wait. boost::xtime xt; BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); - xt.nsec += 100000000; + xt.sec += 10; while (data->notified != 3) data->condition.timed_wait(lock, xt); BOOST_CHECK(lock); @@ -82,11 +82,14 @@ namespace // Test predicate timed_wait. BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); - xt.sec += 2; - BOOST_CHECK(data->condition.timed_wait(lock, xt, cond_predicate(data->notified, 4))); + xt.sec += 10; + cond_predicate pred(data->notified, 4); + BOOST_CHECK(data->condition.timed_wait(lock, xt, pred)); BOOST_CHECK(lock); + BOOST_CHECK(pred()); BOOST_CHECK_EQUAL(data->notified, 4); data->awoken++; + data->condition.notify_one(); } } @@ -165,13 +168,17 @@ void test_condition_waits() while (data.awoken != 3) data.condition.wait(lock); BOOST_CHECK_EQUAL(data.awoken, 3); - } - BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); - xt.sec += 1; - boost::thread::sleep(xt); - data.notified++; - data.condition.notify_one(); + BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); + xt.sec += 1; + boost::thread::sleep(xt); + data.notified++; + data.condition.notify_one(); + while (data.awoken != 4) + data.condition.wait(lock); + BOOST_CHECK_EQUAL(data.awoken, 4); + } + BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); xt.sec += 1; boost::thread::sleep(xt); diff --git a/test/test_mutex.cpp b/test/test_mutex.cpp index 18183ed9..ddf69359 100644 --- a/test/test_mutex.cpp +++ b/test/test_mutex.cpp @@ -6,6 +6,23 @@ #include #include +namespace +{ + inline bool xtime_in_range(boost::xtime& xt, int less_seconds, int greater_seconds) + { + boost::xtime cur; + BOOST_CHECK_EQUAL(boost::xtime_get(&cur, boost::TIME_UTC), boost::TIME_UTC); + + boost::xtime less = cur; + less.sec += less_seconds; + + boost::xtime greater = cur; + greater.sec += greater_seconds; + + return (boost::xtime_cmp(xt, less) >= 0) && (boost::xtime_cmp(xt, greater) <= 0); + } +} + template struct test_lock { @@ -128,6 +145,7 @@ struct test_timedlock // time out. BOOST_CHECK(!condition.timed_wait(lock, xt)); BOOST_CHECK(lock); + BOOST_CHECK(xtime_in_range(xt, -1, 0)); // Test the lock, unlock and timedlock methods. lock.unlock(); diff --git a/test/test_thread.cpp b/test/test_thread.cpp index ee8dfe96..52cf2adf 100644 --- a/test/test_thread.cpp +++ b/test/test_thread.cpp @@ -54,12 +54,12 @@ void test_sleep() { boost::xtime xt; BOOST_CHECK_EQUAL(boost::xtime_get(&xt, boost::TIME_UTC), boost::TIME_UTC); - xt.sec += 5; + xt.sec += 3; boost::thread::sleep(xt); // Insure it's in a range instead of checking actual equality due to time lapse - BOOST_CHECK(xtime_in_range(xt, -1, 1)); + BOOST_CHECK(xtime_in_range(xt, -1, 0)); } void test_creation() diff --git a/test/test_xtime.cpp b/test/test_xtime.cpp index f6c48431..84588eaf 100644 --- a/test/test_xtime.cpp +++ b/test/test_xtime.cpp @@ -5,7 +5,7 @@ void test_xtime_cmp() { boost::xtime xt1, xt2, cur; - boost::xtime_get(&cur, boost::TIME_UTC); + BOOST_CHECK_EQUAL(boost::xtime_get(&cur, boost::TIME_UTC), boost::TIME_UTC); xt1 = xt2 = cur; xt1.nsec -= 1; @@ -24,11 +24,27 @@ void test_xtime_cmp() BOOST_CHECK(boost::xtime_cmp(cur, cur) == 0); } +void test_xtime_get() +{ + boost::xtime orig, cur, old; + BOOST_CHECK_EQUAL(boost::xtime_get(&orig, boost::TIME_UTC), boost::TIME_UTC); + old = orig; + + for (int x=0; x < 100; ++x) + { + BOOST_CHECK_EQUAL(boost::xtime_get(&cur, boost::TIME_UTC), boost::TIME_UTC); + BOOST_CHECK(boost::xtime_cmp(cur, orig) >= 0); + BOOST_CHECK(boost::xtime_cmp(cur, old) >= 0); + old = cur; + } +} + boost::unit_test_framework::test_suite* init_unit_test_suite(int argc, char* argv[]) { boost::unit_test_framework::test_suite* test = BOOST_TEST_SUITE("Boost.Threads: xtime test suite"); test->add(BOOST_TEST_CASE(&test_xtime_cmp)); + test->add(BOOST_TEST_CASE(&test_xtime_get)); return test; } \ No newline at end of file