Skip to content

Conversation

@kevinGC
Copy link
Contributor

@kevinGC kevinGC commented Mar 17, 2023

This test can fail unnecessarily. In the test we wait for events on two file descriptors. This is done in a single call to select.epoll's poll() function. However, it is valid for the OS to return only one event via poll() and the next via a subsequent call to poll(). This rarely happens, but it can cause the test to fail despite properly functioning polling.

Instead, we poll a second time when necessary.

Fixes #102795.

This test can fail unnecessarily. In the test we wait for events on two
file descriptors. This is done in a single call to select.epoll's poll()
function. However, it is valid for the OS to return only one event via
poll() and the next via a subsequent call to poll(). This rarely
happens, but it can cause the test to fail despite properly functioning
polling.

Instead, we poll a second time when necessary.

Fixes python#102795.
@bedevere-bot bedevere-bot added the tests Tests in the Lib/test dir label Mar 17, 2023
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Mar 17, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@arhadthedev
Copy link
Member

@vstinner (as a committer into Modules/selectmodule.c the changed test belongs to)

@vstinner
Copy link
Member

vstinner commented Apr 5, 2023

Hum, maybe loop on calling this function until it returns even events? Maybe loop on test.support.busy_retry(test.support.SHORT_TIMEOUT) to fail if it takes too long.

@kevinGC
Copy link
Contributor Author

kevinGC commented Apr 10, 2023

Updated, PTAL.

@kevinGC
Copy link
Contributor Author

kevinGC commented Apr 26, 2023

Hey, anything I can do to help move this?

@vstinner vstinner enabled auto-merge (squash) May 4, 2023 14:37
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner vstinner merged commit c9ecd3e into python:main May 4, 2023
@vstinner vstinner added the needs backport to 3.11 only security fixes label May 4, 2023
@miss-islington
Copy link
Contributor

Thanks @kevinGC for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-104173 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 4, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 4, 2023
pythonGH-102796)

This test can fail unnecessarily. In the test we wait for events on two
file descriptors. This is done in a single call to select.epoll's poll()
function. However, it is valid for the OS to return only one event via
poll() and the next via a subsequent call to poll(). This rarely
happens, but it can cause the test to fail despite properly functioning
polling.

Instead, we poll a second time when necessary.
(cherry picked from commit c9ecd3e)

Co-authored-by: Kevin Krakauer <[email protected]>
@vstinner
Copy link
Member

vstinner commented May 4, 2023

Merged, thanks. I created a backport to Python 3.11.

@vstinner
Copy link
Member

vstinner commented May 6, 2023

I didn't backport the fix since Python 3.11 doesn't have busy_wait().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_epoll.test_control_and_wait flakes from improper poll() usage

5 participants