GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() by vstinner · Pull Request #28662 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Sep 30, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

On Unix, if the sem_clockwait() function is available in the C
library (glibc 2.30 and newer), the threading.Lock.acquire() method
now uses the monotonic clock (time.CLOCK_MONOTONIC) for the timeout,
rather than using the system clock (time.CLOCK_REALTIME), to not be
affected by system clock changes.

configure now checks if the sem_clockwait() function is available.

https://bugs.python.org/issue41710

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner
bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait()
058de0e
On Unix, if the sem_clockwait() function is available in the C
library (glibc 2.30 and newer), the threading.Lock.acquire() method
now uses the monotonic clock (time.CLOCK_MONOTONIC) for the timeout,
rather than using the system clock (time.CLOCK_REALTIME), to not be
affected by system clock changes.

configure now checks if the sem_clockwait() function is available.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Sep 30, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Sep 30, 2021
vstinner
vstinner commented Sep 30, 2021
View reviewed changes
Comment thread Python/thread_pthread.h Outdated
if (timeout > 0) {
#ifdef HAVE_SEM_CLOCKWAIT
struct timespec abs_timeout;
_PyTime_AsTimespec_clamp(deadline, &abs_timeout);

@vstinner vstinner Sep 30, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't know if sem_clockwait() timeout argument can be modified by the function, for example if the wait is interrupted by a signal. In case of doubt, I prefer to compute abs_timeout before each call.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Sep 30, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In the same file, pthread_cond_timedwait() absolute timeout is only computed once outside the loop. I understand that we can also compute abs_timeout once outside the loop.

The new implementation of time.sleep(), pysleep() function, now also uses an absolute timeout (timespec) computed once outside the loop. It uses clock_nanosleep() or nanosleep(). These functions have an argument for the remaining time, it's not used by pysleep().

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Sep 30, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

sem_clockwait() documentation doesn't say that the absolute timeout is modified, which confirms that it can be only computed once:
https://www.gnu.org/software/libc/manual/html_node/Waiting-with-Explicit-Clocks.html

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
vstinner
vstinner commented Sep 30, 2021
View reviewed changes
Comment thread Python/thread_pthread.h Outdated
#ifdef HAVE_SEM_CLOCKWAIT
struct timespec abs_timeout;
_PyTime_AsTimespec_clamp(deadline, &abs_timeout);
status = fix_status(sem_clockwait(thelock, CLOCK_MONOTONIC, &abs_timeout));

@vstinner vstinner Sep 30, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't know if sem_clockwait() can fail with ENOSYS if Python is built with a recent glibc and then run with an older glibc or a different kernel version.

Should we attempt to fallback to sem_timedwait() if sem_clockwait() "doesn't work"? How can we determine if sem_clockwait() "doesn't work"? So far, I failed to find a sem_clockwait() documentation or manual page.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Sep 30, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

In case of doubt, I prefer to leave the code as it is, and only implement a fallback once we can test it on a real system.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner

vstinner commented Sep 30, 2021

Copy link
Copy Markdown
Member Author

cc @Livius90 @corona10

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
Compute the absolute timeout once outside the loop
ced6f76
@vstinner

vstinner commented Sep 30, 2021

Copy link
Copy Markdown
Member Author

cc @pablogsal @gpshead

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gpshead
gpshead approved these changes Oct 1, 2021
View reviewed changes
Comment thread Python/thread_pthread.h Outdated
break;
}

// sem_clockwait() uses an absolution timeout, there is no need

@gpshead gpshead Oct 1, 2021

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

s/absolution/absolute

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@vstinner vstinner Oct 1, 2021

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nobody expects the Spanish Absolution!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Oct 1, 2021
@vstinner
Fix typo
6c4774e
corona10
corona10 approved these changes Oct 1, 2021
View reviewed changes
@vstinner

vstinner commented Oct 1, 2021

Copy link
Copy Markdown
Member Author

@gpshead @corona10: Thanks for your reviews ;-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
vstinner merged commit 1ee0f94 into python:main Oct 1, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label Oct 1, 2021
@vstinner
vstinner deleted the sem_clockwait branch October 1, 2021 07:55
@duaneg duaneg mentioned this pull request Apr 19, 2025
Python threading.Event().wait() depends on the system time #91394
Closed
@duaneg duaneg mentioned this pull request May 24, 2025
Queue get timout parameter does not use a monotonic timer #104509
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
@corona10 corona10 corona10 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@vstinner @gpshead @corona10 @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.