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

Don't unlink shared memory on Unix #13060

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HazardousPeach
Copy link

@HazardousPeach HazardousPeach commented Sep 10, 2024

Essentially adds the functionality from PR #6091 to Unix platforms. This allows third party tools like DME and JuniorsToolbox to access the shared memory region allowing them to manipulate game memory for various functionality. To do this, keep the shared memory object mapped to a name so that the other process can access it, instead of unlinking it from the name right after creation. This might leak files into the temp filesystem, like mentioned in
#9834 (comment), but will only be one virtual file per dolphin that gets deleted on shutdown, so I think it's fine.

If the leaking is a problem or there's another reason why it's important to unlink the file most of the time, I could try to make a larger pull request that makes this a flag instead, but since it makes the behavior more consistent with windows I thought I'd try it this way first.

Happy to discuss/improve!

Essentially adds the funcitonality from PR dolphin-emu#6091 to Unix
platforms. This allows third party tools like DME and JuniorsToolbox
to access the shared memory region allowing them to manipulate game
memory for various functionality. To do this, keep the shared memory
object mapped to a name so that the other process can access it,
instead of unlinking it from the name right after creation. This might
leak files into the filesystem, like mentioned in
dolphin-emu#9834 (comment),
but will only be one virtual file per dolphin so I think it's fine.
@NerduMiner
Copy link

I think it would make sense to move the unlink call to MemArena::ReleaseSHMSegment(). There'd have to be a way to retain the name of the segment for that function though.

@HazardousPeach
Copy link
Author

Added a commit which stores the most recently acquired memory segment name in a member variable on the MemArena and uses that to clean up on ReleaseSHMSegment. I figure that's safe because the file descriptor for the memory segment is also stored in a member variable on every acquire, so it's already not safe to Grab twice without Releasing in between.

@JMC47
Copy link
Contributor

JMC47 commented Oct 4, 2024

I'm not against something like this for programs like DME, but I also am not the right person to review it. Considering it's such a small change, hopefully someone can give this thing a look so we can know if it's the right way to go about it.

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

Successfully merging this pull request may close these issues.

3 participants