Remove bad code from ComputeRelativeTimeSpec
The test in the "else if" clause was off by one. But it should never be
exercised anyway. Replace it with a correct DCHECK.
Have ComputeRelativeTimeSpec return false, rather than true, on failure
to compute a valid timespec.
Update num_contenders if lock acquisition results in unexpected
failure. This matters only in that it may put us into a slightly
better state while logging debug information.
Fix an unrelated comment typo.
Bug: 238032384
Test: Build and boot AOSP eng build
Change-Id: Idf3a0b4c349168370079d24457ea134b06cf231e
diff --git a/runtime/alloc_instrumentation.md b/runtime/alloc_instrumentation.md
index 513bbe3..66e9a6a 100644
--- a/runtime/alloc_instrumentation.md
+++ b/runtime/alloc_instrumentation.md
@@ -17,7 +17,7 @@
- These in turn are called by `SetStatsEnabled()`, `SetAllocationListener()`, et al, which
require the mutator lock is not held.
-- With a started runtime, `SetEntrypointsInstrumented()` calls `ScopedSupendAll(`) before updating
+- With a started runtime, `SetEntrypointsInstrumented()` calls `ScopedSuspendAll(`) before updating
the function table.
Mutual exclusion in the dispatch table is thus ensured by the fact that it is only updated while
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 5709333..01d7e73 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -59,18 +59,19 @@
};
#if ART_USE_FUTEXES
+// Compute a relative timespec as *result_ts = lhs - rhs.
+// Return false (and produce an invalid *result_ts) if lhs < rhs.
static bool ComputeRelativeTimeSpec(timespec* result_ts, const timespec& lhs, const timespec& rhs) {
const int32_t one_sec = 1000 * 1000 * 1000; // one second in nanoseconds.
+ static_assert(std::is_signed<decltype(result_ts->tv_sec)>::value); // Signed on Linux.
result_ts->tv_sec = lhs.tv_sec - rhs.tv_sec;
result_ts->tv_nsec = lhs.tv_nsec - rhs.tv_nsec;
if (result_ts->tv_nsec < 0) {
result_ts->tv_sec--;
result_ts->tv_nsec += one_sec;
- } else if (result_ts->tv_nsec > one_sec) {
- result_ts->tv_sec++;
- result_ts->tv_nsec -= one_sec;
}
- return result_ts->tv_sec < 0;
+ DCHECK(result_ts->tv_nsec >= 0 && result_ts->tv_nsec < one_sec);
+ return result_ts->tv_sec >= 0;
}
#endif
@@ -852,7 +853,7 @@
timespec now_abs_ts;
InitTimeSpec(true, CLOCK_MONOTONIC, 0, 0, &now_abs_ts);
timespec rel_ts;
- if (ComputeRelativeTimeSpec(&rel_ts, end_abs_ts, now_abs_ts)) {
+ if (!ComputeRelativeTimeSpec(&rel_ts, end_abs_ts, now_abs_ts)) {
return false; // Timed out.
}
ScopedContentionRecorder scr(this, SafeGetTid(self), GetExclusiveOwnerTid());
@@ -869,6 +870,7 @@
// EAGAIN and EINTR both indicate a spurious failure,
// recompute the relative time out from now and try again.
// We don't use TEMP_FAILURE_RETRY so we can recompute rel_ts;
+ num_contenders_.fetch_sub(1); // Unlikely to matter.
PLOG(FATAL) << "timed futex wait failed for " << name_;
}
}