westphahl commented 3 years ago

The lock must only consider contenders with a sequence number lower than it's own sequence number as also stated in the Zookeeper recipe description for shared locks.

This wasn't working correctly as the ReadLock also considered WriteLocks with a higher sequence number as contenders. This can lead to a deadlock as described in #649.

Closes #649

westphahl commented 3 years ago

Is it possible to add a test case for #649?

Done.

StephenSorriaux commented 3 years ago

Thank you, I will first fix the CI since travis-ci.org has ceased and then take a look at your PR.

westphahl commented 2 years ago

@StephenSorriaux I rebased the fix in order to have the CI jobs running.

codecov-commenter commented 2 years ago

Codecov Report

Merging #650 (884cac3) into master (4042a85) will increase coverage by 0.04%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #650      +/-   ##
==========================================
+ Coverage   94.44%   94.49%   +0.04%     
==========================================
  Files          57       57              
  Lines        8342     8375      +33     
==========================================
+ Hits         7879     7914      +35     
+ Misses        463      461       -2

Impacted Files	Coverage Δ
kazoo/recipe/lock.py	`97.96% <100.00%> (+<0.01%)`	:arrow_up:
kazoo/tests/test_lock.py	`99.30% <100.00%> (-0.15%)`	:arrow_down:
kazoo/protocol/connection.py	`95.25% <0.00%> (+0.20%)`	:arrow_up:
kazoo/client.py	`98.56% <0.00%> (+0.31%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4042a85...884cac3. Read the comment docs.

StephenSorriaux commented 2 years ago

Mh seems like Codecov has some trouble... Will retry the jobs tomorow

python-zk / kazoo

fix(recipe): fix deadlock in r/w lock recipe #650

Codecov Report