Remind periodically while a context lock acquisition is blocked - #2105
Remind periodically while a context lock acquisition is blocked#2105fzyzcjy wants to merge 1 commit into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
2143343 to
eed5783
Compare
7a70205 to
d2eb99a
Compare
5fcfecc to
e3ed318
Compare
402aad1 to
37462a1
Compare
e3ed318 to
7238aa6
Compare
37462a1 to
546ecc7
Compare
7238aa6 to
3020b4c
Compare
546ecc7 to
7ce2b5b
Compare
3de206d to
b0bf4fd
Compare
60d4450 to
febcc22
Compare
b0bf4fd to
5a6ee2a
Compare
febcc22 to
b74a0f6
Compare
7674e53 to
eb6615e
Compare
b8b0c18 to
625f9aa
Compare
bf5e911 to
f48e7de
Compare
625f9aa to
ee8fe50
Compare
f48e7de to
9c02fbc
Compare
4e8269b to
eb82f68
Compare
c0cf3b5 to
7970a67
Compare
0094c14 to
f8d66e9
Compare
7970a67 to
bda3e1e
Compare
f8d66e9 to
e7e2e8e
Compare
bda3e1e to
e5dc24d
Compare
e7e2e8e to
ccb6823
Compare
e5dc24d to
2befcbd
Compare
ccb6823 to
0754207
Compare
cf5cb7e to
8cf2805
Compare
8183120 to
c02c940
Compare
8cf2805 to
4c6b14a
Compare
c02c940 to
752d4fa
Compare
4c6b14a to
61b94ca
Compare
| async def acquire(self) -> None: | ||
| assert _held_lock.get() is None, f"Cannot acquire lock {self._name!r}: a context lock is already held" | ||
| await self._lock.acquire() | ||
| wait_reminder = asyncio.ensure_future(self._remind_while_waiting()) |
There was a problem hiding this comment.
[P2][unresolved at safe checkpoint #2176 and checked tip #2649] Please avoid spawning a reminder task on every uncontended acquire. Because asyncio.Lock.acquire() can complete without yielding, a tight acquire/release sequence leaves one cancel-requested task and ready callback per call until the loop runs again; on this head, 100,000 calls retained about 91.8 MB before the next yield. Please preserve an allocation-free uncontended path and reliably drain the contended-path reminder without weakening lock fairness or cancellation safety.
|
|
||
| waiter = asyncio.create_task(lock.acquire()) | ||
| with caplog.at_level(logging.INFO, logger="miles.utils.context_lock"): | ||
| await asyncio.sleep(0.08) |
There was a problem hiding this comment.
[P2][unresolved at safe checkpoint #2176 and checked tip #2649] Please wait on observed reminder events instead of assuming two 10 ms timers run within this 80 ms window. Under a busy or stalled CI event loop, the test sleep and first reminder can become runnable together, so the waiter is cancelled after only one log and this test flakes despite correct reminder behavior; a deterministic 100 ms event-loop stall reproduces it on this head.
752d4fa to
dcc0741
Compare
61b94ca to
d38b682
Compare
Part of #1837