Skip to content

AsyncServiceBrowser must recheck for handlers to call when holding condition#478

Closed
bdraco wants to merge 1 commit into
python-zeroconf:masterfrom
bdraco:handlers_to_call_asb_race
Closed

AsyncServiceBrowser must recheck for handlers to call when holding condition#478
bdraco wants to merge 1 commit into
python-zeroconf:masterfrom
bdraco:handlers_to_call_asb_race

Conversation

@bdraco

@bdraco bdraco commented Jun 6, 2021

Copy link
Copy Markdown
Member
  • There was a short race condition window where the AsyncServiceBrowser
    could add to _handlers_to_call in the Engine thread, have the
    condition notify_all called, but since the AsyncServiceBrowser was
    not yet holding the condition it would not know to stop waiting
    and process the handlers to call.

Fixes #476 for async, #477 is the fix for sync

Fixes #471

@codecov-commenter

codecov-commenter commented Jun 6, 2021

Copy link
Copy Markdown

Codecov Report

Merging #478 (0945a1b) into master (b853413) will increase coverage by 0.26%.
The diff coverage is 93.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
+ Coverage   95.62%   95.88%   +0.26%     
==========================================
  Files           4        4              
  Lines        3338     3381      +43     
  Branches      402      409       +7     
==========================================
+ Hits         3192     3242      +50     
+ Misses         89       83       -6     
+ Partials       57       56       -1     
Impacted Files Coverage Δ
zeroconf/asyncio.py 97.40% <91.22%> (-1.32%) ⬇️
zeroconf/__init__.py 92.66% <100.00%> (+0.64%) ⬆️
zeroconf/test.py 98.88% <100.00%> (+<0.01%) ⬆️

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 ed53f62...0945a1b. Read the comment docs.

@bdraco bdraco force-pushed the handlers_to_call_asb_race branch from cdccef2 to 423bc12 Compare June 6, 2021 21:11
@bdraco

bdraco commented Jun 6, 2021

Copy link
Copy Markdown
Member Author

Except asyncio.wait_for on an asyncio.Condition is broken https://bugs.python.org/issue39032

@bdraco bdraco force-pushed the handlers_to_call_asb_race branch 2 times, most recently from 144a194 to ecd6ff3 Compare June 6, 2021 22:27
…ndition

- There was a short race condition window where the AsyncServiceBrowser
  could add to _handlers_to_call in the Engine thread, have the
  condition notify_all called, but since the AsyncServiceBrowser was
  not yet holding the condition it would not know to stop waiting
  and process the handlers to call.
@bdraco bdraco force-pushed the handlers_to_call_asb_race branch from ecd6ff3 to 66bf28f Compare June 6, 2021 22:32
@bdraco

bdraco commented Jun 6, 2021

Copy link
Copy Markdown
Member Author

...and the test failure is in the sync code that is fixed by #477

@bdraco

bdraco commented Jun 6, 2021

Copy link
Copy Markdown
Member Author

Its all working now, but I need to break this into two PRs

  1. Switch to using .condition
  2. AsyncServiceBrowser race fix

@bdraco bdraco closed this Jun 6, 2021
@bdraco bdraco deleted the handlers_to_call_asb_race branch June 6, 2021 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants