Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-43895: Remove an unnecessary cache of shared object handles #25487

Merged
merged 2 commits into from Jul 7, 2021

Conversation

insertinterestingnamehere
Copy link
Contributor

@insertinterestingnamehere insertinterestingnamehere commented Apr 20, 2021

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@insertinterestingnamehere

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@merwok merwok changed the title bpo:43895: Remove an unnecessary cache of shared object handles. bpo-43895: Remove an unnecessary cache of shared object handles Apr 23, 2021
@pitrou pitrou requested a review from gpshead May 13, 2021 21:15
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jun 13, 2021
@gpshead gpshead requested a review from Yhg1s July 6, 2021 03:53
@gpshead
Copy link
Member

gpshead commented Jul 6, 2021

This old handles cache code appears to have been added in 1995 via fbe6d33

I agree, I'm not sure why we'd need it anymore. Adding @Yhg1s for additional eyeballs and possible historical context.

It looks like it might have been a workaround to avoid attempting to dlopen the same .so multiple times in the presence of symlinks or hardlinks? Why should we even need to do that?

@Yhg1s
Copy link
Member

Yhg1s commented Jul 6, 2021

The cache isn't an optimisation measure, it was added in 1995 (in fbe6d33) to ensure the same file doesn't get dlopened twice. (It's not a particularly strong guarantee, because of the fixed size array, and it's certainly not hard to imagine importing more than 128 extension modules nowadays. I'm pretty sure there weren't 128 extension modules total in 1995. :)

I know glibc does its own caching, and that behaviour is guaranteed in the dlopen manpage. All other OS manpages for dlopen I could find (FreeBSd, OpenBSD, NetBSD, zOS, Solaris, HPUX, SCO Unix) mention it as well. I'm not sure if it's mandated by any standards, but at this point it hard to imagine a posix-y system not behaving that way. We never dlclose() extension modules, so the handle cache isn't there to prevent problems when unloading and loading the same extension module again, either.

Removing this for 3.11 should be fine. (Add a NEWS entry mentioning it, though.)

@gpshead gpshead assigned gpshead and unassigned Yhg1s Jul 7, 2021
@gpshead gpshead added needs backport to 3.10 only security fixes and removed needs backport to 3.10 only security fixes labels Jul 7, 2021
@insertinterestingnamehere
Copy link
Contributor Author

Sounds great! Thanks for reviewing and merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants