Skip to content

Commit

Permalink
remove lock blocking tracking (#1944)
Browse files Browse the repository at this point in the history 
the feature was never really used downstream, and is removed from there.
  • Loading branch information
@mikea
mikea committed May 13, 2024
1 parent 8dd8823 commit d0c1ad5
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 480 deletions.
17 changes: 0 additions & 17 deletions .github/workflows/quick-test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,6 @@ jobs:
export CC=clang-${{ matrix.clang }}
export CXX=clang++-${{ matrix.clang }}
${{ matrix.run-test }}
Linux-lock-tracking:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
compiler: [clang-14]
features: ["-DKJ_TRACK_LOCK_BLOCKING=1 -DKJ_SAVE_ACQUIRED_LOCK_INFO=1 -DKJ_CONTENTION_WARNING_THRESHOLD=200"]
steps:
- uses: actions/checkout@v4
- name: install dependencies
run: |
export DEBIAN_FRONTEND=noninteractive
sudo apt-get install -y build-essential git zlib1g-dev cmake libssl-dev ${{ matrix.compiler }}
- name: super-test ${{ matrix.compiler }}
run: |
# librt is used for timer_create in the unit tests for lock tracking (mutex-test.c++).
./super-test.sh quick ${{ matrix.compiler }} cpp-features "${{matrix.features}}" extra-libs "-lrt"
MacOS:
runs-on: macos-latest
strategy:
Expand Down
16 changes: 0 additions & 16 deletions c++/build/configure.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ def kj_configure():
build_setting_default = False,
)

bool_flag(
name = "save_acquired_lock_info",
build_setting_default = False,
)

bool_flag(
name = "track_lock_blocking",
build_setting_default = False,
)

bool_flag(
name = "deprecate_kj_if_maybe",
build_setting_default = True,
Expand Down Expand Up @@ -103,12 +93,6 @@ def kj_configure():
}) + select({
"//src/kj:use_libdl": ["KJ_HAS_LIBDL"],
"//conditions:default": [],
}) + select({
"//src/kj:use_save_acquired_lock_info": ["KJ_SAVE_ACQUIRED_LOCK_INFO=1"],
"//conditions:default": ["KJ_SAVE_ACQUIRED_LOCK_INFO=0"],
}) + select({
"//src/kj:use_track_lock_blocking": ["KJ_TRACK_LOCK_BLOCKING=1"],
"//conditions:default": ["KJ_TRACK_LOCK_BLOCKING=0"],
}) + select({
"//src/kj:use_deprecate_kj_if_maybe": ["KJ_DEPRECATE_KJ_IF_MAYBE=1"],
"//conditions:default": ["KJ_DEPRECATE_KJ_IF_MAYBE=0"],
Expand Down
232 changes: 0 additions & 232 deletions c++/src/kj/mutex-test.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,6 @@
#include <vector>
#endif

#if KJ_TRACK_LOCK_BLOCKING
#include <syscall.h>
#include <signal.h>
#include <time.h>
#include <atomic>
#endif

namespace kj {
namespace {

Expand Down Expand Up @@ -638,231 +631,6 @@ KJ_TEST("condvar wait with flapping predicate") {
}
}

#if KJ_TRACK_LOCK_BLOCKING
#if !__GLIBC_PREREQ(2, 30)
#ifndef SYS_gettid
#error SYS_gettid is unavailable on this system
#endif

#define gettid() ((pid_t)syscall(SYS_gettid))
#endif

KJ_TEST("tracking blocking on mutex acquisition") {
// SIGEV_THREAD is supposed to be "private" to the pthreads implementation, but, as
// usual, the higher-level POSIX API that we're supposed to use sucks: the "handler" runs on
// some other thread, which means the stack trace it prints won't be useful.
//
// So, we cheat and work around libc.
MutexGuarded<int> foo(5);
auto lock = foo.lockExclusive();

struct BlockDetected {
volatile bool blockedOnMutexAcquisition;
SourceLocation blockLocation;
} blockingInfo = {};

struct sigaction handler;
memset(&handler, 0, sizeof(handler));
handler.sa_sigaction = [](int, siginfo_t* info, void*) {
auto& blockage = *reinterpret_cast<BlockDetected *>(info->si_value.sival_ptr);
KJ_IF_SOME(r, blockedReason()) {
KJ_SWITCH_ONEOF(r) {
KJ_CASE_ONEOF(b, BlockedOnMutexAcquisition) {
blockage.blockedOnMutexAcquisition = true;
blockage.blockLocation = b.origin;
}
KJ_CASE_ONEOF_DEFAULT {}
}
}
};
handler.sa_flags = SA_SIGINFO | SA_RESTART;

sigaction(SIGINT, &handler, nullptr);

timer_t timer;
struct sigevent event;
memset(&event, 0, sizeof(event));
event.sigev_notify = SIGEV_THREAD_ID;
event.sigev_signo = SIGINT;
event.sigev_value.sival_ptr = &blockingInfo;
KJ_SYSCALL(event._sigev_un._tid = gettid());
KJ_SYSCALL(timer_create(CLOCK_MONOTONIC, &event, &timer));
KJ_DEFER(timer_delete(timer));

kj::Duration timeout = 50 * MILLISECONDS;
struct itimerspec spec;
memset(&spec, 0, sizeof(spec));
spec.it_value.tv_sec = timeout / kj::SECONDS;
spec.it_value.tv_nsec = timeout % kj::SECONDS / kj::NANOSECONDS;
// We can't use KJ_SYSCALL() because it is not async-signal-safe.
KJ_REQUIRE(-1 != timer_settime(timer, 0, &spec, nullptr));

kj::SourceLocation expectedBlockLocation;
KJ_REQUIRE(foo.lockSharedWithTimeout(100 * MILLISECONDS, expectedBlockLocation) == kj::none);

KJ_EXPECT(blockingInfo.blockedOnMutexAcquisition);
KJ_EXPECT(blockingInfo.blockLocation == expectedBlockLocation);
}

KJ_TEST("tracking blocked on CondVar::wait") {
// SIGEV_THREAD is supposed to be "private" to the pthreads implementation, but, as
// usual, the higher-level POSIX API that we're supposed to use sucks: the "handler" runs on
// some other thread, which means the stack trace it prints won't be useful.
//
// So, we cheat and work around libc.
MutexGuarded<int> foo(5);
auto lock = foo.lockExclusive();

struct BlockDetected {
volatile bool blockedOnCondVar;
SourceLocation blockLocation;
} blockingInfo = {};

struct sigaction handler;
memset(&handler, 0, sizeof(handler));
handler.sa_sigaction = [](int, siginfo_t* info, void*) {
auto& blockage = *reinterpret_cast<BlockDetected *>(info->si_value.sival_ptr);
KJ_IF_SOME(r, blockedReason()) {
KJ_SWITCH_ONEOF(r) {
KJ_CASE_ONEOF(b, BlockedOnCondVarWait) {
blockage.blockedOnCondVar = true;
blockage.blockLocation = b.origin;
}
KJ_CASE_ONEOF_DEFAULT {}
}
}
};
handler.sa_flags = SA_SIGINFO | SA_RESTART;

sigaction(SIGINT, &handler, nullptr);

timer_t timer;
struct sigevent event;
memset(&event, 0, sizeof(event));
event.sigev_notify = SIGEV_THREAD_ID;
event.sigev_signo = SIGINT;
event.sigev_value.sival_ptr = &blockingInfo;
KJ_SYSCALL(event._sigev_un._tid = gettid());
KJ_SYSCALL(timer_create(CLOCK_MONOTONIC, &event, &timer));
KJ_DEFER(timer_delete(timer));

kj::Duration timeout = 50 * MILLISECONDS;
struct itimerspec spec;
memset(&spec, 0, sizeof(spec));
spec.it_value.tv_sec = timeout / kj::SECONDS;
spec.it_value.tv_nsec = timeout % kj::SECONDS / kj::NANOSECONDS;
// We can't use KJ_SYSCALL() because it is not async-signal-safe.
KJ_REQUIRE(-1 != timer_settime(timer, 0, &spec, nullptr));

SourceLocation waitLocation;

lock.wait([](const int& value) {
return false;
}, 100 * MILLISECONDS, waitLocation);

KJ_EXPECT(blockingInfo.blockedOnCondVar);
KJ_EXPECT(blockingInfo.blockLocation == waitLocation);
}

KJ_TEST("tracking blocked on Once::init") {
// SIGEV_THREAD is supposed to be "private" to the pthreads implementation, but, as
// usual, the higher-level POSIX API that we're supposed to use sucks: the "handler" runs on
// some other thread, which means the stack trace it prints won't be useful.
//
// So, we cheat and work around libc.
struct BlockDetected {
volatile bool blockedOnOnceInit;
SourceLocation blockLocation;
} blockingInfo = {};

struct sigaction handler;
memset(&handler, 0, sizeof(handler));
handler.sa_sigaction = [](int, siginfo_t* info, void*) {
auto& blockage = *reinterpret_cast<BlockDetected *>(info->si_value.sival_ptr);
KJ_IF_SOME(r, blockedReason()) {
KJ_SWITCH_ONEOF(r) {
KJ_CASE_ONEOF(b, BlockedOnOnceInit) {
blockage.blockedOnOnceInit = true;
blockage.blockLocation = b.origin;
}
KJ_CASE_ONEOF_DEFAULT {}
}
}
};
handler.sa_flags = SA_SIGINFO | SA_RESTART;

sigaction(SIGINT, &handler, nullptr);

timer_t timer;
struct sigevent event;
memset(&event, 0, sizeof(event));
event.sigev_notify = SIGEV_THREAD_ID;
event.sigev_signo = SIGINT;
event.sigev_value.sival_ptr = &blockingInfo;
KJ_SYSCALL(event._sigev_un._tid = gettid());
KJ_SYSCALL(timer_create(CLOCK_MONOTONIC, &event, &timer));
KJ_DEFER(timer_delete(timer));

Lazy<int> once;
MutexGuarded<bool> onceInitializing(false);

Thread backgroundInit([&] {
once.get([&](SpaceFor<int>& x) {
*onceInitializing.lockExclusive() = true;
usleep(100 * 1000); // 100 ms
return x.construct(5);
});
});

kj::Duration timeout = 50 * MILLISECONDS;
struct itimerspec spec;
memset(&spec, 0, sizeof(spec));
spec.it_value.tv_sec = timeout / kj::SECONDS;
spec.it_value.tv_nsec = timeout % kj::SECONDS / kj::NANOSECONDS;
// We can't use KJ_SYSCALL() because it is not async-signal-safe.
KJ_REQUIRE(-1 != timer_settime(timer, 0, &spec, nullptr));

kj::SourceLocation onceInitializingBlocked;

onceInitializing.lockExclusive().wait([](const bool& initializing) {
return initializing;
});

once.get([](SpaceFor<int>& x) {
return x.construct(5);
}, onceInitializingBlocked);

KJ_EXPECT(blockingInfo.blockedOnOnceInit);
KJ_EXPECT(blockingInfo.blockLocation == onceInitializingBlocked);
}

#if KJ_SAVE_ACQUIRED_LOCK_INFO
KJ_TEST("get location of exclusive mutex") {
_::Mutex mutex;
kj::SourceLocation lockAcquisition;
mutex.lock(_::Mutex::EXCLUSIVE, kj::none, lockAcquisition);
KJ_DEFER(mutex.unlock(_::Mutex::EXCLUSIVE));

const auto& lockedInfo = mutex.lockedInfo();
const auto& lockInfo = lockedInfo.get<_::HoldingExclusively>();
EXPECT_EQ(gettid(), lockInfo.threadHoldingLock());
KJ_EXPECT(lockInfo.lockAcquiredAt() == lockAcquisition);
}

KJ_TEST("get location of shared mutex") {
_::Mutex mutex;
kj::SourceLocation lockLocation;
mutex.lock(_::Mutex::SHARED, kj::none, lockLocation);
KJ_DEFER(mutex.unlock(_::Mutex::SHARED));

const auto& lockedInfo = mutex.lockedInfo();
const auto& lockInfo = lockedInfo.get<_::HoldingShared>();
KJ_EXPECT(lockInfo.lockAcquiredAt() == lockLocation);
}
#endif

#endif

#ifdef KJ_CONTENTION_WARNING_THRESHOLD
KJ_TEST("make sure contended mutex warns") {
class Expectation final: public ExceptionCallback {
Expand Down
64 changes: 1 addition & 63 deletions c++/src/kj/mutex.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,29 +59,7 @@
#endif

namespace kj {
#if KJ_TRACK_LOCK_BLOCKING
static thread_local const BlockedOnReason* tlsBlockReason __attribute((tls_model("initial-exec")));
// The initial-exec model ensures that even if this code is part of a shared library built PIC, then
// we still place this variable in the appropriate ELF section so that __tls_get_addr is avoided.
// It's unclear if __tls_get_addr is still not async signal safe in glibc. The only negative
// downside of this approach is that a shared library built with kj & lock tracking will fail if
// dlopen'ed which isn't an intended use-case for the initial implementation.

Maybe<const BlockedOnReason&> blockedReason() noexcept {
if (tlsBlockReason == nullptr) {
return kj::none;
}
return *tlsBlockReason;
}

static void setCurrentThreadIsWaitingFor(const BlockedOnReason* meta) {
tlsBlockReason = meta;
}

static void setCurrentThreadIsNoLongerWaiting() {
tlsBlockReason = nullptr;
}
#elif KJ_USE_FUTEX
#ifdef KJ_USE_FUTEX
struct BlockedOnMutexAcquisition {
constexpr BlockedOnMutexAcquisition(const _::Mutex& mutex, LockSourceLocationArg) {}
};
Expand Down Expand Up @@ -179,33 +157,6 @@ struct timespec toAbsoluteTimespec(TimePoint time) {
// =======================================================================================
// Futex-based implementation (Linux-only)

#if KJ_SAVE_ACQUIRED_LOCK_INFO
#if !__GLIBC_PREREQ(2, 30)
#ifndef SYS_gettid
#error SYS_gettid is unavailable on this system
#endif

#define gettid() ((pid_t)syscall(SYS_gettid))
#endif

static thread_local pid_t tlsTid = gettid();
#define TRACK_ACQUIRED_TID() tlsTid

Mutex::AcquiredMetadata Mutex::lockedInfo() const {
auto state = __atomic_load_n(&futex, __ATOMIC_RELAXED);
auto tid = lockedExclusivelyByThread;
auto location = lockAcquiredLocation;

if (state & EXCLUSIVE_HELD) {
return HoldingExclusively{tid, location};
} else {
return HoldingShared{location};
}
}

#else
#define TRACK_ACQUIRED_TID() 0
#endif

Mutex::Mutex(): futex(0) {}
Mutex::~Mutex() {
Expand Down Expand Up @@ -257,7 +208,6 @@ bool Mutex::lock(Exclusivity exclusivity, Maybe<Duration> timeout, LockSourceLoc
}
}
}
acquiredExclusive(TRACK_ACQUIRED_TID(), location);
#if KJ_CONTENTION_WARNING_THRESHOLD
printContendedReader = false;
#endif
Expand Down Expand Up @@ -324,18 +274,6 @@ bool Mutex::lock(Exclusivity exclusivity, Maybe<Duration> timeout, LockSourceLoc
}
#endif

// We just want to record the lock being acquired somewhere but the specific location doesn't
// matter. This does mean that race conditions could occur where a thread might read this
// inconsistently (e.g. filename from 1 lock & function from another). This currently is just
// meant to be a debugging aid for manual analysis so it's OK for that purpose. If it's ever
// required for this to be used for anything else, then this should probably be changed to
// use an additional atomic variable that can ensure only one writer updates this. Or use the
// futex variable to ensure that this is only done for the first one to acquire the lock,
// although there may be thundering herd problems with that whereby there's a long wallclock
// time between when the lock is acquired and when the location is updated (since the first
// locker isn't really guaranteed to be the first one unlocked).
acquiredShared(location);

break;
}
}
Expand Down

0 comments on commit d0c1ad5

Please sign in to comment.