GeezerVison

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

GeezerVison is showing github.com — bpo-46398: posixshmem module shm_rename freebsd support. by devnexen · Pull Request #30621 · python/cpython

Conversation

devnexen commented Jan 16, 2022 •

edited by bedevere-bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

https://bugs.python.org/issue46398

Sorry, something went wrong.

Uh oh!

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

All reactions

the-knights-who-say-ni added the CLA signed label

Jan 16, 2022

bedevere-bot added the awaiting review label

Jan 16, 2022

devnexen changed the title ~~posixshmem module shm_rename freebsd support.~~ bpo-46398: posixshmem module shm_rename freebsd support.

Jan 16, 2022

devnexen force-pushed the shm_rename branch 5 times, most recently from 36de652 to 4bec161 Compare

January 17, 2022 12:49

devnexen marked this pull request as ready for review

January 17, 2022 20:10

devnexen requested a review from tiran as a code owner

January 17, 2022 20:10

github-actions Bot commented Feb 17, 2022

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

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

github-actions Bot added the stale Stale PR or inactive for long period of time. label

Feb 17, 2022

This comment was marked as outdated.

vstinner added the OS-freebsd label

Apr 21, 2022

ezio-melotti removed the CLA signed label

Jul 13, 2022

devnexen mannequin mentioned this pull request

Nov 29, 2023

multiprocessing: add SharedMemory.rename() -- shm_rename() FreeBSD support #90556

Open

serhiy-storchaka reviewed

Feb 24, 2024

View reviewed changes

Comment thread

Modules/_multiprocessing/posixshmem.c Outdated


		Rename a shared memory object.

		Remove a shared memory object and relink to another path

serhiy-storchaka Feb 24, 2024

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.

Suggested change

	Remove a shared memory object and relink to another path
	Remove a shared memory object and relink to another path.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Modules/_multiprocessing/posixshmem.c

		_posixshmem.shm_rename
		path_from: unicode
		path_to: unicode
		flags: int

serhiy-storchaka Feb 24, 2024

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.

Suggested change

	flags: int
	flags: int
	/

I think it was mistake to make parameters keyword-or-positional in other functions. It is better to make them positional-only, it will help if we decide to rename parameters.

Sorry, something went wrong.

Uh oh!

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

erlend-aasland reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

Lib/multiprocessing/shared_memory.py Outdated

		With the SHM_RENAME_NOREPLACE flag, an error will be returned
		if the new name exists.
		"""
		if platform.system() != "FreeBSD":

serhiy-storchaka Feb 24, 2024

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.

It is better to use hasattr(). If in future shm_rename() be added on other platforms, it will automatically add support of SharedMemory.rename().

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Modules/_multiprocessing/posixshmem.c Outdated

Comment on lines +90 to +101

		const char *from = PyUnicode_AsUTF8(path_from);
		const char *to = PyUnicode_AsUTF8(path_to);
		if (from == NULL \|\| to == NULL) {
		return NULL;
		}

serhiy-storchaka Feb 24, 2024

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.

This function has the same bug as shm_open() and shm_unlink() (see #115886). Please check the names for embedded null characters.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/multiprocessing/shared_memory.py Outdated

		if newname:
		newname = "/" + newname if self._prepend_leading_slash else newname
		r = _posixshmem.shm_rename(self._name, newname, flags)
		if r == None:

serhiy-storchaka Feb 24, 2024

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.

It is always true, isn't?

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Lib/multiprocessing/shared_memory.py

		if platform.system() != "FreeBSD":
		raise OSError("Unsupported operation on this platform")

		if newname:

serhiy-storchaka Feb 24, 2024

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.

So it simply returns False if newname is empty and returns True otherwise? Why add such check in this method?

Sorry, something went wrong.

Uh oh!

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

All reactions

devnexen and others added 4 commits

February 24, 2024 20:32


          posixshmem module shm_rename freebsd support.

4ecc7bd


          update shared_memory library

1336f8c


          doc update

d11ff82


          📜🤖 Added by blurb_it.

13734b6

devnexen force-pushed the shm_rename branch from 50bf4c3 to 13734b6 Compare

February 24, 2024 21:07

devnexen requested review from corona10, erlend-aasland and gpshead as code owners

February 24, 2024 21:07

vstinner reviewed

May 24, 2024

View reviewed changes

Comment thread

Lib/multiprocessing/shared_memory.py

		With the SHM_RENAME_NOREPLACE flag, an error will be returned
		if the new name exists.
		"""
		if !platform.hasattr("shm_rename"):

vstinner May 24, 2024

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.

Suggested change

	if !platform.hasattr("shm_rename"):
	if not platform.hasattr("shm_rename"):

Sorry, something went wrong.

Uh oh!

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

All reactions

vstinner May 24, 2024

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.

What is this platform.hasattr() function? Where does it come from?

Sorry, something went wrong.

Uh oh!

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

All reactions

aisk mentioned this pull request

Apr 5, 2025

gh-90556: add SharedMemory.rename on multiprocessing.shared_memory #132130

Open

github-actions Bot removed the stale Stale PR or inactive for long period of time. label

Apr 10, 2026

github-actions Bot commented May 11, 2026

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

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

github-actions Bot added the stale Stale PR or inactive for long period of time. label

May 11, 2026

vstinner commented May 13, 2026

Copy link

Copy Markdown

Member

The PR is idle for 2 years, the author didn't reply to reviews. I close the PR.

All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner closed this

May 13, 2026

Reviewers

vstinner

vstinner left review comments

serhiy-storchaka

serhiy-storchaka left review comments

tiran

Awaiting requested review from tiran

gpshead

Awaiting requested review from gpshead

gpshead is a code owner

erlend-aasland

Awaiting requested review from erlend-aasland

erlend-aasland is a code owner

corona10

Awaiting requested review from corona10

corona10 is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review OS-freebsd stale

Stale PR or inactive for long period of time.

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.

7 participants

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.

Conversation

devnexen commented Jan 16, 2022 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 17, 2022

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

vstinner commented May 13, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

devnexen commented Jan 16, 2022 •

edited by bedevere-bot

Loading

Uh oh!

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