Skip to content

select: Don't try selecting on a closed event loop#431

Closed
Marenz wants to merge 1 commit into
frequenz-floss:v1.x.xfrom
Marenz:eventloo
Closed

select: Don't try selecting on a closed event loop#431
Marenz wants to merge 1 commit into
frequenz-floss:v1.x.xfrom
Marenz:eventloo

Conversation

@Marenz

@Marenz Marenz commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

With this change, we don't try to clean up any tasks (after the eventloop has ended) as we don't even try to create a task and don't enter the try/finally block if there is no event loop at all.

Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
Copilot AI review requested due to automatic review settings June 19, 2025 14:04
@Marenz Marenz requested a review from a team as a code owner June 19, 2025 14:04
@github-actions github-actions Bot added the part:synchronization Affects the synchronization of multiple sources (`select`, `merge`) label Jun 19, 2025

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR prevents the select operation from running on a closed asyncio event loop by checking the event loop's state and raising an appropriate error.

  • Adds a check in the select() function to confirm that the event loop is open before proceeding.
  • Raises a SelectError when the event loop is closed, ensuring proper error handling.

receivers_map: dict[str, Receiver[Any]] = {str(hash(r)): r for r in receivers}
pending: set[asyncio.Task[bool]] = set()

if asyncio.get_event_loop().is_closed():

Copilot AI Jun 19, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider using 'asyncio.get_running_loop()' instead of 'asyncio.get_event_loop()' for improved clarity and compatibility with newer Python versions where a running loop is expected.

Suggested change
if asyncio.get_event_loop().is_closed():
if asyncio.get_running_loop().is_closed():

Copilot uses AI. Check for mistakes.
@Marenz Marenz added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jun 19, 2025
@Marenz

Marenz commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

closing, I don't think it actually helps

@Marenz Marenz closed this Jun 19, 2025
@Marenz Marenz deleted the eventloo branch June 19, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:synchronization Affects the synchronization of multiple sources (`select`, `merge`)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants