codeant-ai-for-open-source[bot] commented on code in PR #43678:
URL: https://github.com/apache/superset/pull/43678#discussion_r3885531217
##########
superset/coordination/types.py:
##########
@@ -38,22 +38,30 @@ class SignalListener:
"""Handle for a background listener started by
:meth:`~superset.coordination.base.CoordinationService.listen_for_signal`.
- Wraps the daemon thread and its stop flag. :meth:`stop` sets the flag and
joins;
- the listener's bounded blocking read means it notices the flag within one
short
- tick.
+ Wraps the daemon thread and its stop flag. :meth:`stop` sets the flag and,
+ when a ``wake`` is provided, nudges the backend stream so a listener
parked in
+ a blocking read returns at once rather than waiting out its block interval
—
+ keeping task teardown from paying the full read timeout.
"""
def __init__(
self,
thread: threading.Thread,
stop_event: threading.Event,
+ wake: "Callable[[], None] | None" = None,
) -> None:
self._thread = thread
self._stop_event = stop_event
+ self._wake = wake
def stop(self) -> None:
"""Signal the listener to stop and wait briefly for the thread to
finish."""
self._stop_event.set()
+ # Wake a listener blocked in a backend read so it observes the stop
flag
+ # immediately (the no-backend loop already wakes on the event).
Best-effort:
+ # a failed nudge just falls back to the bounded join below.
+ if self._wake is not None:
+ self._wake()
Review Comment:
**Suggestion:** The wake callback performs synchronous Redis `xadd` and
`expire` calls before the bounded `join`. If the coordination backend is
degraded or has no effective socket timeout, `self._wake()` can block
indefinitely, so `stop()` never reaches its two-second join or daemon-thread
fallback. Run the nudge with a bounded timeout or invoke it asynchronously
while preserving the bounded teardown guarantee. [resource leak]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Task teardown can block on degraded Redis.
- ⚠️ Worker capacity can be consumed by stuck teardown calls.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9f3f3d0e36b84b54a563e4fb6c7aab62&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9f3f3d0e36b84b54a563e4fb6c7aab62&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/coordination/types.py
**Line:** 63:64
**Comment:**
*Resource Leak: The wake callback performs synchronous Redis `xadd` and
`expire` calls before the bounded `join`. If the coordination backend is
degraded or has no effective socket timeout, `self._wake()` can block
indefinitely, so `stop()` never reaches its two-second join or daemon-thread
fallback. Run the nudge with a bounded timeout or invoke it asynchronously
while preserving the bounded teardown guarantee.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43678&comment_hash=9d8cf83151e06abf53e64b63fb3b7a15c1b76b0735f0a022e72f0c1b8c34307a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43678&comment_hash=9d8cf83151e06abf53e64b63fb3b7a15c1b76b0735f0a022e72f0c1b8c34307a&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
