Skip to content

Conversation

matthew-mcallister
Copy link

Pull Request check-list

Please make sure to review and check all of these items:

  • Do tests and lints pass with this change?
  • Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • Is there an example added to the examples folder (if applicable)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Description of change

This implements the reader-write lock requested in #2372. It is a pretty sizeable standalone feature that may take a while to review.

The interface and implementation I think are explained decently well in docstrings and comments. Briefly: the lock state is stored in three keys:

  1. <prefix>:write: Standard mutex lease
  2. <prefix>:write_semaphore: Semaphore that tracks waiting writers, implemented as an ordered set.
  3. <prefix>:read: Semaphore that tracks readers.

I tried to keep the interface (and implementation) similar to the existing Lock class while taking inspiration from the Rust and C++ standard libraries for the new bits. I got rid of thread-local state since each read/write guard now generates its own token.

Other than supporting multiple readers, the only really "new" feature is the max_writers parameter, which can be used to cap the number of waiting writers. Typically I expect most users would only choose 0/None (unlimited) or 1 (guarantee single writer).

Benchmark stuff

Other than tests, I included a "benchmark" which is in reality a simulation of the use-case that I originally wanted this feature for. It simulates a group of workers reading a cached value with expiration. When the cached value expires, the workers coordinate so only one tries to replace the cached value at a time. It also tries to verify that no concurrency errors occurred.

The simulation adds dev-dependencies on pandas (for computing min/mean/p95/max) and matplotlib (for reproducing graphs). The matplotlib dependency is not necessary to run the benchmark, however.

I primarily wrote this simulation to provide empirical evidence of the correctness and viability of my design and implementation. Admittedly, I don't think the quantities it measures are very meaningful as-is from a rigorous performance perspective, so I'll leave it to maintainers to decide if it should go in or be omitted/replaced with a more useful benchmark.

Sample run output
n = 1, ttl = 0.05
iops: 43.40
                      min   mean    p95    max
read_acquire_time  0.09ms 0.16ms 0.20ms 0.34ms
read_release_time  0.07ms 0.12ms 0.15ms 0.28ms
write_acquire_time 0.10ms 0.15ms 0.19ms 0.28ms
write_release_time 0.10ms 0.15ms 0.19ms 0.20ms
                      success  timeout  aborted
read_acquire_status       217        0        0
write_acquire_status       51        0        0

n = 1, ttl = 0.1 iops: 38.20 min mean p95 max read_acquire_time 0.10ms 0.16ms 0.20ms 0.29ms read_release_time 0.07ms 0.12ms 0.16ms 0.26ms write_acquire_time 0.10ms 0.15ms 0.21ms 0.24ms write_release_time 0.11ms 0.15ms 0.20ms 0.21ms success timeout aborted read_acquire_status 191 0 0 write_acquire_status 35 0 0
n = 1, ttl = 0.25 iops: 37.80 min mean p95 max read_acquire_time 0.10ms 0.17ms 0.20ms 0.42ms read_release_time 0.08ms 0.13ms 0.15ms 0.20ms write_acquire_time 0.11ms 0.16ms 0.21ms 0.24ms write_release_time 0.13ms 0.16ms 0.20ms 0.27ms success timeout aborted read_acquire_status 189 0 0 write_acquire_status 17 0 0
n = 1, ttl = 0.5 iops: 37.60 min mean p95 max read_acquire_time 0.11ms 0.16ms 0.20ms 0.45ms read_release_time 0.08ms 0.13ms 0.18ms 0.25ms write_acquire_time 0.12ms 0.15ms 0.18ms 0.18ms write_release_time 0.14ms 0.16ms 0.20ms 0.20ms success timeout aborted read_acquire_status 188 0 0 write_acquire_status 10 0 0
n = 1, ttl = 1 iops: 36.00 min mean p95 max read_acquire_time 0.10ms 0.15ms 0.19ms 0.26ms read_release_time 0.08ms 0.12ms 0.15ms 0.20ms write_acquire_time 0.12ms 0.14ms 0.18ms 0.18ms write_release_time 0.12ms 0.15ms 0.18ms 0.19ms success timeout aborted read_acquire_status 180 0 0 write_acquire_status 5 0 0
n = 2, ttl = 0.05 iops: 46.60 min mean p95 max read_acquire_time 0.09ms 9.32ms 50.66ms 202.22ms read_release_time 0.07ms 0.12ms 0.17ms 0.27ms write_acquire_time 0.12ms 37.43ms 77.26ms 101.27ms write_release_time 0.12ms 0.15ms 0.18ms 0.25ms success timeout aborted read_acquire_status 237 0 0 write_acquire_status 40 0 1
n = 2, ttl = 0.1 iops: 59.40 min mean p95 max read_acquire_time 0.09ms 4.64ms 50.59ms 101.32ms read_release_time 0.07ms 0.12ms 0.18ms 0.27ms write_acquire_time 0.12ms 35.72ms 87.45ms 126.39ms write_release_time 0.12ms 0.15ms 0.18ms 0.19ms success timeout aborted read_acquire_status 299 0 0 write_acquire_status 32 0 0
n = 2, ttl = 0.25 iops: 68.00 min mean p95 max read_acquire_time 0.10ms 2.37ms 0.43ms 101.48ms read_release_time 0.07ms 0.12ms 0.15ms 0.31ms write_acquire_time 0.11ms 31.03ms 76.03ms 76.13ms write_release_time 0.10ms 0.15ms 0.17ms 0.27ms success timeout aborted read_acquire_status 342 0 0 write_acquire_status 18 0 0
n = 2, ttl = 0.5 iops: 75.80 min mean p95 max read_acquire_time 0.09ms 1.41ms 0.20ms 75.82ms read_release_time 0.07ms 0.11ms 0.15ms 0.28ms write_acquire_time 0.16ms 32.99ms 50.69ms 50.70ms write_release_time 0.12ms 0.15ms 0.22ms 0.27ms success timeout aborted read_acquire_status 380 0 0 write_acquire_status 10 0 0
n = 2, ttl = 1 iops: 75.40 min mean p95 max read_acquire_time 0.09ms 0.68ms 0.20ms 50.66ms read_release_time 0.07ms 0.12ms 0.15ms 0.28ms write_acquire_time 0.12ms 33.83ms 50.69ms 50.70ms write_release_time 0.09ms 0.13ms 0.17ms 0.18ms success timeout aborted read_acquire_status 378 0 0 write_acquire_status 6 0 0
n = 4, ttl = 0.05 iops: 77.80 min mean p95 max read_acquire_time 0.09ms 19.54ms 101.03ms 177.08ms read_release_time 0.07ms 0.11ms 0.14ms 0.29ms write_acquire_time 0.16ms 55.46ms 126.33ms 126.78ms write_release_time 0.09ms 0.14ms 0.17ms 0.28ms success timeout aborted read_acquire_status 397 0 0 write_acquire_status 37 0 7
n = 4, ttl = 0.1 iops: 102.00 min mean p95 max read_acquire_time 0.10ms 10.62ms 75.96ms 151.66ms read_release_time 0.08ms 0.13ms 0.17ms 0.53ms write_acquire_time 0.12ms 54.32ms 76.12ms 101.18ms write_release_time 0.11ms 0.15ms 0.19ms 0.22ms success timeout aborted read_acquire_status 515 0 0 write_acquire_status 28 0 4
n = 4, ttl = 0.25 iops: 117.20 min mean p95 max read_acquire_time 0.10ms 5.78ms 50.67ms 151.57ms read_release_time 0.07ms 0.11ms 0.16ms 0.24ms write_acquire_time 0.11ms 61.76ms 107.93ms 126.69ms write_release_time 0.11ms 0.14ms 0.16ms 0.16ms success timeout aborted read_acquire_status 588 0 0 write_acquire_status 16 0 1
n = 4, ttl = 0.5 iops: 145.40 min mean p95 max read_acquire_time 0.10ms 2.30ms 0.27ms 100.94ms read_release_time 0.07ms 0.12ms 0.17ms 0.47ms write_acquire_time 0.21ms 43.13ms 75.99ms 76.05ms write_release_time 0.12ms 0.15ms 0.20ms 0.23ms success timeout aborted read_acquire_status 731 0 0 write_acquire_status 10 0 3
n = 4, ttl = 1 iops: 139.40 min mean p95 max read_acquire_time 0.09ms 1.39ms 0.21ms 126.29ms read_release_time 0.07ms 0.12ms 0.15ms 0.31ms write_acquire_time 0.13ms 54.96ms 95.01ms 101.28ms write_release_time 0.11ms 0.13ms 0.15ms 0.15ms success timeout aborted read_acquire_status 699 0 0 write_acquire_status 6 0 1
n = 8, ttl = 0.05 iops: 124.80 min mean p95 max read_acquire_time 0.10ms 33.85ms 151.49ms 252.28ms read_release_time 0.07ms 0.12ms 0.18ms 0.70ms write_acquire_time 0.60ms 78.43ms 126.55ms 177.43ms write_release_time 0.10ms 0.14ms 0.19ms 0.21ms success timeout aborted read_acquire_status 637 0 0 write_acquire_status 31 0 12
n = 8, ttl = 0.1 iops: 183.00 min mean p95 max read_acquire_time 0.09ms 15.83ms 101.17ms 177.15ms read_release_time 0.07ms 0.12ms 0.18ms 0.63ms write_acquire_time 0.16ms 71.30ms 118.84ms 126.45ms write_release_time 0.09ms 0.15ms 0.23ms 0.25ms success timeout aborted read_acquire_status 927 0 0 write_acquire_status 27 0 11
n = 8, ttl = 0.25 iops: 227.40 min mean p95 max read_acquire_time 0.09ms 8.15ms 75.96ms 176.72ms read_release_time 0.07ms 0.13ms 0.19ms 0.65ms write_acquire_time 0.12ms 64.92ms 107.47ms 126.36ms write_release_time 0.12ms 0.16ms 0.19ms 0.20ms success timeout aborted read_acquire_status 1140 0 0 write_acquire_status 16 0 2
n = 8, ttl = 0.5 iops: 264.60 min mean p95 max read_acquire_time 0.09ms 3.60ms 1.26ms 151.48ms read_release_time 0.06ms 0.11ms 0.15ms 0.95ms write_acquire_time 0.38ms 63.29ms 115.09ms 126.43ms write_release_time 0.10ms 0.13ms 0.15ms 0.15ms success timeout aborted read_acquire_status 1328 0 0 write_acquire_status 10 0 4
n = 8, ttl = 1 iops: 279.00 min mean p95 max read_acquire_time 0.09ms 2.46ms 0.25ms 177.03ms read_release_time 0.07ms 0.12ms 0.17ms 0.75ms write_acquire_time 0.26ms 67.59ms 101.39ms 101.44ms write_release_time 0.11ms 0.14ms 0.17ms 0.17ms success timeout aborted read_acquire_status 1399 0 0 write_acquire_status 6 0 3
image image image image image image image image image image image image image image image image image image image image

Copy link

jit-ci bot commented Oct 2, 2025

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@petyaslavova
Copy link
Collaborator

Hi @matthew-mcallister thank you for your contribution! We will review your change soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants