Skip to content

Add SelfContainedMutex#2386

Merged
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:scmutex
Sep 2, 2022
Merged

Add SelfContainedMutex#2386
sporksmith merged 2 commits intoshadow:mainfrom
sporksmith:scmutex

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith commented Aug 26, 2022

This is a Mutex suitable for use in shared memory.

  • Has a fixed layout (repr(C))
  • Self-contained (no pointers)
  • Works across processes (e.g. doesn't use FUTEX_PRIVATE_FLAG)

I've also implemented rkyv::Archive and rkyv::Serialize for it. This facilitates using it inside of types meant to be serialized with rkyv. That in turn may be useful for safely managing shared memory.

@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging labels Aug 26, 2022
@sporksmith sporksmith marked this pull request as ready for review August 26, 2022 22:26
@sporksmith
Copy link
Copy Markdown
Contributor Author

@stevenengler friendly ping; lmk if you'd like to chat about this one or anything

Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

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

Looks really cool! I'm not the best person to look for memory safety stuff, but I added some comments for the things that I saw.

@sporksmith
Copy link
Copy Markdown
Contributor Author

Might be useful to look at one commit at a time - I added a fixup commit responding directly to feedback, and another addressing potential Pin safety issues

Copy link
Copy Markdown
Contributor Author

@sporksmith sporksmith left a comment

Choose a reason for hiding this comment

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

Oops, forgot to actually send my responses

Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

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

I can't review the Pin stuff since I don't know much about it. I do think it would be nice to have a better solution for the "UnsafeCell within a immutable buffer" issue, but I know you're actively in discussion about that, so I don't want to hold back this PR. But regardless it looks pretty cool and interested to see where it goes :)

This is a Mutex suitable for use in shared memory.

 * Has a fixed layout (repr(C))
 * Self-contained (no pointers)
 * Works across processes (e.g. doesn't use FUTEX_PRIVATE_FLAG)
@github-actions github-actions bot added the Component: Main Composing the core Shadow executable label Sep 2, 2022
@sporksmith
Copy link
Copy Markdown
Contributor Author

After rebasing, one of the tests broke in units.rs: https://github.com/shadow/shadow/runs/8160879195?check_suite_focus=true#step:3:328

I can't figure out how it's breaking in this PR but not in main, but the fix is trivial so I just went ahead and added it

@sporksmith sporksmith enabled auto-merge September 2, 2022 17:34
@sporksmith sporksmith merged commit 58598ee into shadow:main Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants