Use memfd_create and shm_open instead of mkstemp #140

dlrobertson · 2017-02-09T03:59:39Z

Remove the use of mkstemp which creates a file in /tmp. Instead use the following

linux - memfd_create
freebsd - shm_open

highfive · 2017-02-09T03:59:42Z

Warning

These commits modify unsafe code. Please review it carefully!

dlrobertson · 2017-02-09T04:02:11Z

src/platform/unix/mod.rs

@@ -982,6 +970,13 @@ unsafe fn create_shmem(name: *const c_char, length: usize) -> c_int {
    fd
 }

+#[cfg(target_os="android")]
+unsafe fn create_shmem(name: *const c_char, length: usize) -> c_int {
+    let fd = ashmem_create_region(name, length);


NB: I have not tested the android code yet. This is more or less a placeholder for now.

Tested this today, and it appears to work.

Manishearth · 2017-02-09T04:02:22Z

YES! Thank you!

For mac there are some Mach calls that are really good for this

cc #125

r? @antrik @pcwalton

dlrobertson · 2017-02-09T04:09:09Z

src/platform/unix/mod.rs

@@ -1033,6 +1058,11 @@ fn is_socket(fd: c_int) -> bool {

 // FFI stuff follows:

+#[cfg(target_os="linux")]
+unsafe fn memfd_create(name: *const c_char, flags: usize) -> c_int {
+    syscall!(MEMFD_CREATE, name, flags) as c_int


Kind of less than ideal in my books, but since glibc doesn't provide an wrapper, this is the recommended method. Perhaps we should make shm_open the default and this enabled by a feature flag?

That's odd... Do you know whether glibc just doesn't provide a wrapper yet, or it doesn't intend to? (And if so, why?)

I don't think it's a reason to disable it by default though. It certainly would be useful however to have a --force-posix-shm flag along the lines of --force-inprocess, so we can easily test the code locally and in CI.

I see that you already did that in fact :-)

dlrobertson · 2017-02-09T04:55:55Z

src/platform/unix/mod.rs

@@ -649,10 +649,69 @@ fn make_socket_lingering(sockfd: c_int) -> Result<(),UnixError> {
    Ok(())
 }

+struct BackingStore {


In my original design, this played a much bigger role. The BackingStore doesn't really do much as is, and could easily be removed. We really rely on the create_shmem function.

While it didn't end up being platform-specific -- which I suppose was the reason you introduced it -- I still think it's a useful abstraction. Dealing with raw FDs is always quite fragile; anything that isolates this is good.

antrik · 2017-02-09T14:45:45Z

@Manishearth the dedicated MacOS back-end already uses Mach memory objects. This is only about platforms using the Unix back-end.

Manishearth · 2017-02-09T16:31:46Z

already uses Mach memory objects.

TIL, nice.

antrik

This looks pretty good already :-) (And steals an item from my ToDo list -- except that I probably wouldn't have bothered adding anything but POSIX SHM myself...)

My major qualm is that the changes are in a sub-optimal order: causing a lot of unnecessary code churn, and making review of individual commits very weird... It would be much more logical to introduce the generic POSIX SHM implementation first, completely replacing the temp file one in one clean atomic commit; and only later bringing in the more platform-specific variants.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

+}
+
+impl BackingStore {
+    pub unsafe fn create(length: usize) -> BackingStore {


This method doesn't need to be unsafe -- there is nothing the caller can do to break stuff by invoking it. Only the implementation is unsafe.

(Admittedly, this was also true of the original stand-alone function... Maybe you could fix this first in a separate commit -- not crucial though...)

Also, since this is the primary constructor method, I believe convention demands it to be called new()...

Good point. I renamed the function to new and moved the code inside of an unsafe block. I probably could have moved only parts of the code inside the unsafe block, but I thought this looked more readable. I'm okay with changing that though.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

+
+    pub fn from_fd(fd: c_int) -> BackingStore {
+        BackingStore {
+            fd: fd


Nitpick: always terminate the last line in a multi-line initialiser with , as well.

(I just noticed that I didn't catch the same thing in the mio PR...)

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

@@ -649,10 +649,69 @@ fn make_socket_lingering(sockfd: c_int) -> Result<(),UnixError> {
    Ok(())
 }

+struct BackingStore {


While it didn't end up being platform-specific -- which I suppose was the reason you introduced it -- I still think it's a useful abstraction. Dealing with raw FDs is always quite fragile; anything that isolates this is good.

antrik · 2017-02-09T20:23:12Z

Cargo.toml

@@ -8,6 +8,7 @@ repository = "https://github.com/servo/ipc-channel"

 [features]
 force-inprocess = []
+force-shm = []


Probably best to add this in whatever commit first introduces the more specific variants.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

-
-#[cfg(not(target_os="android"))]
-const TEMP_FILE_TEMPLATE: &'static str = "/tmp/ipc-channel-shared-memory.XXXXXX";
+const TEMP_FILE_TEMPLATE: &'static str = "/ipc-channel-shared-memory.";


The name is not really fitting now...

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

-unsafe fn maybe_unlink(c: *const c_char) -> c_int {
-    libc::unlink(c)
+#[cfg(any(feature="force-shm", target_os="freebsd"))]
+#[inline]


Explicit inlining is useful only for functions invoked from other crates.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

+        let fd = create_shmem(string.as_ptr(), length);
+        BackingStore {
+            fd: fd
+        }


Could use Self::from_fd() I guess? Not sure whether that's indeed more readable though...

Debated this for a while, but I went ahead and also updated this.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

-    0
+unsafe fn create_filename() -> CString {
+    let mut tmp_string = TEMP_FILE_TEMPLATE.to_owned();
+    let addition: String = thread_rng().gen_ascii_chars().take(6).collect();


@asajeffrey spent some effort removing use of thread_rng() in Servo, since it uses up an FD for each thread...

There is actually no need for this to be random -- so I think a global lazy_static! atomic counter would do just fine. It would be less overhead, too.

(In fact I'm not sure there is any value in using unique names with memfd at all: the name is only for debugging here; and since we don't keep track of it after initial creation of the FD anyway, I don't think they really help telling the FDs apart...)

Seems more tricky for the POSIX SHM interface though, since the names are global there. Probably need to use the PID along with a counter and timestamp. (We need the timestamp part, since theoretically a process might die before releasing the SHM object, and later another process reusing the PID might collide with that...)

I still have some work to do on this. In the meantime I removed the use of thread_rng() in this function and used a combination of the pid and timestamp. To be completely honest, I totally forgot to add the counter 😄

Seems more tricky for the POSIX SHM interface though

Yes, AFAIK both of the other platforms should be fine with the lazy_static, but it gets a lot trickier with any platforms using the shm_open implementation.

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

+#[cfg(any(feature="force-shm", target_os="freebsd"))]
+#[inline]
+unsafe fn create_shmem(name: *const c_char, length: usize) -> c_int {
+    let fd = libc::shm_open(name, libc::O_CREAT | libc::O_RDWR, 0660);


Why 660, not 600?

(Though in fact it probably should be 000, since we don't ever open the same object again anyway -- we just clone the existing FD, which is already open with O_RDWR...)

Also, 0660 doesn't do what you think it does :-) In Rust, you'd need 0o660.

Last but not least, it's probably good to add O_EXCL just to be safe.

Updated.

Also, 0660 doesn't do what you think it does :-) In Rust, you'd need 0o660.

Old habits die hard

antrik · 2017-02-09T20:23:12Z

src/platform/unix/mod.rs

+unsafe fn create_filename() -> CString {
+    let mut tmp_string = TEMP_FILE_TEMPLATE.to_owned();
+    let addition: String = thread_rng().gen_ascii_chars().take(6).collect();
+    tmp_string.push_str(&addition);


I think it would be easier to use format!() to create the string?

antrik · 2017-02-09T20:40:18Z

Just to be clear: it's especially the second commit that is weird, since it refactors code that is obsoleted piece by piece by later commits... It would be much cleaner to do any refactoring necessary on top of the POSIX SHM code as the new base-line, before adding the specific Linux and Android back-ends.

dlrobertson · 2017-02-10T00:57:13Z

My major qualm is that the changes are in a sub-optimal order: causing a lot of unnecessary code churn, and making review of individual commits very weird...

Good feedback! Reordered the commits and condensed a few commits.

antrik

The commit message of the third commit doesn't fit any more -- mkstemp() is already gone at this point :-) (Good riddance...)

(BTW, using "Android platform" and "Linux platform" might be slightly misleading in this context, since these are just special cases of the Unix platform module...)

antrik · 2017-02-10T15:42:48Z

.travis.yml

@@ -10,6 +10,7 @@ os:
 env:
  - FEATURES="unstable"
  - FEATURES="unstable force-inprocess"
+  - FEATURES="unstable force-posix-shm"


I wonder whether there is a way to enable this only on the Linux builder? It makes no sense on MacOS...

Yeah, I though that was a little strange... I'l look into this

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

-    // and threading that value into Servo.
-    // https://code.google.com/p/android/issues/detail?id=19017
-    0
+fn create_string() -> CString {


create_string() doesn't really have a lot of meaning... Should be something like create_shm_name() I'd say -- though actually I'm not convinced it's useful to have a separate function for this at all...

though actually I'm not convinced it's useful to have a separate function for this at all

Originally I was going to generate the name differently for different OSes, but in the end I thought it was probably best to keep it simple. I'll add the name creation to the BackingStore::create method.

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

-    // https://code.google.com/p/android/issues/detail?id=19017
-    0
+fn create_string() -> CString {
+    let epoc = UNIX_EPOCH.elapsed().unwrap_or(Duration::new(0, 0));


I think you can just unwrap() here -- time elapsed since epoch should always be valid.

(Weird that there seems to be no simpler way to obtain this... And also weird that there seems to be no way to obtain a global monotonic timestamp. Doesn't really matter though in this case, as with a precise timestamp along with the PID and a counter, a collision is well-nigh impossible even if system time does jump back...)

epoc is a weird variable name, BTW... Just timestamp I'd say?

I'll update this tomorrow

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

+    let epoc = UNIX_EPOCH.elapsed().unwrap_or(Duration::new(0, 0));
+    let pid = unsafe {
+        libc::getpid()
+    };


For such a simple unsafe expression, I believe it's more customary to put it on a single line.

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

+    let pid = unsafe {
+        libc::getpid()
+    };
+    CString::new(&*format!("/ipc-channel-shared-memory.{}.{}.{}",


You don't need the &* dance here: CString::new() actually wants an owned string (or some other byte vector) -- if you turn it into a slice first, this just means it will have to clone it unnecessarily to gain ownership...

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

-unsafe fn maybe_unlink(c: *const c_char) -> c_int {
-    libc::unlink(c)
+#[cfg(any(feature="force-posix-shm", not(any(target_os="linux", target_os="android"))))]
+unsafe fn create_shmem(name: *const c_char, length: usize) -> c_int {


This wouldn't need to be unsafe, if you pass name as a CString.

Good point! That is definitely a win

antrik · 2017-02-10T15:42:48Z

src/platform/unix/mod.rs

-    libc::unlink(c)
+#[cfg(any(feature="force-posix-shm", not(any(target_os="linux", target_os="android"))))]
+unsafe fn create_shmem(name: *const c_char, length: usize) -> c_int {
+    let fd = libc::shm_open(name, libc::O_CREAT | libc::O_RDWR | libc::O_EXCL, 0o600);


A quick check seems to confirm that this works with 0o000 as well, as I expected :-)

👍 Nice! I'll change that

I just tested this on FreeBSD and unfortunately you can't unlink if you don't open with at least 0o600

dlrobertson · 2017-02-11T21:12:06Z

.travis.yml

+      env: FEATURES="unstable"
+    - os: osx
+      rust: nightly
+      env: FEATURES="unstable force-inprocess"


This is a bit more verbose, but it doesn't build with force-posix-shm on osx.

Since this is a somewhat major change, I wonder whether it might be better to do it in a separate commit?

This is now the last commit

dlrobertson · 2017-02-11T21:15:32Z

Updated to address comments from previous review. I also removed the OS specific commit for android. From some continued testing and code review, I realized that the OS specific code for android under the unix module is unreachable since the inprocess implementation is used. I'll submit an issue and possibly work on a follow-up PR that makes android use the unix module.

antrik · 2017-02-12T03:18:11Z

src/platform/unix/mod.rs

+        let pid = unsafe { libc::getpid() };
+        let name = CString::new(format!("/ipc-channel-shared-memory.{}.{}.{}",
+                                        count, pid,
+                                        timestamp.as_secs())).unwrap();


Why did you drop the nanosecond part? It seems perfectly plausible for a process to exec() (thus resetting the counter while keeping the PID), and then create new shared memory objects all within a single second...

With a more precise timestamp on the other hand, the time it takes to exec() should be sufficient to avoid collisions.

(To be honest, it feels a bit fragile even with that, if running on a system with low timer resolution... I see no obvious way to fix this though, without re-introducing some sort of random element :-( )

Why did you drop the nanosecond part? It seems perfectly plausible for a process to exec() (thus resetting the counter while keeping the PID), and then create new shared memory objects all within a single second...

I didn't think about the exec scenario. This is definitely the part of the PR that I'm most unsure about at the moment. I guess if we go back to the pid, counter, second, nanosecond combo I'd be happy with it, but I also started to wonder if some sort of random hash might be better...

Nanosec part was added back in. So now we have /ipc-channel-shared-memory.<count>.<pid>.<seconds>.<nanoseconds>

I'm really uncertain myself whether a random hash wouldn't be preferable. I don't know whether other methods of generating random numbers -- such as weak_rng(), or perhaps even StdRng::new() in a lazy_static -- would avoid the FD use problem. If so, it might be preferable, since it's probably a tick simpler all in all... Really not sure though. I don't see a strong motivation to change it at this point.

antrik · 2017-02-12T03:18:11Z

.travis.yml

+      env: FEATURES="unstable"
+    - os: osx
+      rust: nightly
+      env: FEATURES="unstable force-inprocess"


Since this is a somewhat major change, I wonder whether it might be better to do it in a separate commit?

antrik · 2017-02-12T03:18:11Z

src/platform/unix/mod.rs

+    unsafe {
+        let fd = libc::shm_open(name.as_ptr(),
+                                libc::O_CREAT | libc::O_RDWR | libc::O_EXCL,
+                                0o600);


So why didn't you go with 0o200?...

So the FreeBSD man page says shm_unlink needs write privileges, but testing is showing it needs read-write.

Way to keep programmers on their toes ;-)

Probably good to add a comment for this?

Haha so true... Comment added.

antrik · 2017-02-12T03:18:11Z

src/platform/unix/mod.rs

+
+impl BackingStore {
+    pub fn new(length: usize) -> BackingStore {
+        let count = GLOBAL_SHM_COUNT.fetch_add(1, Ordering::SeqCst);


While the documentation on Ordering is unfortunately pretty useless to mere mortals, after doing some digging, I'm fairly confident that Relaxed would do here...

Haha. Yeah, I always forget that Relaxed is the same as LLVM's definition of monotomic.

dlrobertson · 2017-02-13T00:45:14Z

src/platform/unix/mod.rs

@@ -825,7 +880,7 @@ fn recv(fd: c_int, blocking_mode: BlockingMode)
        let bytes_read = try!(cmsg.recv(fd, blocking_mode));
        main_data_buffer.set_len(bytes_read - mem::size_of_val(&total_size));

-        let cmsg_fds = cmsg.cmsg_buffer.offset(1) as *const c_int;
+        let cmsg_fds = CMSG_DATA(cmsg.cmsg_buffer) as *const c_int;


Found another spot we weren't using CMSG_DATA.

That's not really related to this PR though, is it?...

I can submit this as a separate one... There are a few other things I've found that I'll want to fix too

Removed and submitted as #144

antrik · 2017-02-14T22:51:58Z

src/platform/unix/mod.rs

@@ -72,6 +68,8 @@ lazy_static! {
    };
 }

+static GLOBAL_SHM_COUNT: AtomicUsize = ATOMIC_USIZE_INIT;


I totally forgot to comment on this: I don't think the GLOBAL_ prefix is useful. I haven't ever seen something like that used with Rust. (The all-caps already identify it clearly as a global.)

Maybe also good to add a comment that this is used for creating the unique IDs?

Added a comment and removed GLOBAL_. I left the SHM_ prefix and named the lazy_static pid SHM_PID. I don't really like the SHM_ prefix, but I wanted to make it a little clearer what it was used for.

Personally, I don't think PID should have a SHM_ prefix -- while its (currently) used only for this, the PID itself is a general property of the process, not in any way SHM-specific...

I don't feel strongly about it though, and I don't think this warrants another iteration :-)

antrik · 2017-02-14T22:51:58Z

src/platform/unix/mod.rs

+    pub fn new(length: usize) -> BackingStore {
+        let count = GLOBAL_SHM_COUNT.fetch_add(1, Ordering::Relaxed);
+        let timestamp = UNIX_EPOCH.elapsed().unwrap();
+        let pid = unsafe { libc::getpid() };


I wonder whether it would be useful to put this in a lazy_static. I don't know whether this is cached somehow -- but if it isn't, and performs an actual system call every time, this could have a somewhat significant performance impact...

Sorry, you mentioned this earlier but I didn't get around to it. Updated per the above comment.

Nah, I actually only mentioned using lazy_static for the counter before -- not realising that an atomic is the more obvious choice for that one...

dlrobertson · 2017-02-18T18:00:03Z

Yes, that works.

Awesome! PR submitted and trying now.

dlrobertson · 2017-02-18T19:05:03Z

@antrik I should have brought this up sooner, but MEMFD_CREATE wasn't introduced until 3.17. So perhaps we should switch the force-posix-shm flag for a memfd-create features flag. We could also add a build.rs function to check if MEMFD_CREATE exists.

antrik · 2017-02-19T21:13:35Z

I don't think submitting "fake PRs" is a sustainable workflow. Unless we can include Servo testing in ipc-channel CI (which is probably unrealistic), we just can't expect this do be done on every major change. If we don't trust local testing to be sufficient, probably the best we can do is trying to land risky changes in Servo ASAP, so any problems get exposed in a timely fashion.

antrik · 2017-02-19T21:19:56Z

@danlrobertson Linux 3.17 is a pretty steep requirement indeed...

Build-time feature detection is not sufficient, as the official nightlies for example need to be portable. We could attempt run-time detection -- but that would complicate the code, making it more error-prone and harder to test. I wonder whether it's really worthwhile under these conditions? Maybe it's best to make memfd opt-in for now after all...

dlrobertson · 2017-02-19T21:53:05Z

If we don't trust local testing to be sufficient, probably the best we can do is trying to land risky changes in Servo ASAP, so any problems get exposed in a timely fashion.

Agreed. Not having a solid definition of a "major" change is complicates things more (I'm also of the opinion that a "small" change is almost just as likely to introduce problems). IMO keeping ipc-channel "up to date" in servo would be ideal, but managing dependencies like serde could end up being a bit of a pain. If keeping the ipc-channel version at the latest version is too risky, perhaps we could have an ipc-channel branch? That would also be a pain to maintain... but it could reduce the risk on introducing a bug into servo that stalled other devs work?

Maybe it's best to make memfd opt-in for now after all

I'm starting to lean towards the memfd-create feature as well.

antrik · 2017-02-20T14:22:47Z

Just memfd I'd say... The extra verbosity seems more confusing than clarifying in this case.


        Refctor the backing store for OsIpcSharedMemory

Refactor the backing store functionality of OsIpcSharedMemory (create_memory_backing_store, map_file). Create a new structure BackingStore that contains the create and map_file functions which implement the same functionality.


        Use posix shm for unix platforms

Use POSIX shm_open/shm_unlink for unix OSes instead of mkstemp. In addition, move the platform specific code used to create the backing store out into a separate function create_shmem.


        Auto merge of #15632 - danlrobertson:try_shmem_update, r=<try>

[WIP] test run on servo Ignore this PR. This is just a test run for servo/ipc-channel#140.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15632)

dlrobertson · 2017-02-20T20:07:19Z

Updated to include a memfd feature, rebased on master, and updated servo PR.

antrik

The commit message for the last commit needs updating.

(While at it, might be useful to mention why it required switching to the explicit build matrix? Probably quite surprising at first otherwise...)

antrik · 2017-02-21T01:07:40Z

src/platform/unix/mod.rs

-    libc::free(string_buffer as *mut c_void);
-    assert!(libc::ftruncate(fd, length as off_t) == 0);
-    fd
+#[cfg(any(not(feature="memfd"), target_os="freebsd"))]


This should be written in terms of not(target_os="linux"). (It's the default implementation, used in all cases unless we have a more specific one...)

In fact it's probably clearer to write it as #[cfg(not(all(target_os="linux", feature="memfd")))] -- not least because that way it's obvious that it's complementary to the other one.

(I'd put the linux bit before the memfd one, since memfd is dependant on linux... Not really important though.)


        Auto merge of #15632 - danlrobertson:try_shmem_update, r=<try>

[WIP] test run on servo Ignore this PR. This is just a test run for servo/ipc-channel#140.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15632)


        Add memfd feature to use memfd_create for linux

For linux OSes allow the use of memfd_create for the adventurous souls that dare to do so. This feature should be enabled with the memfd features flag and should only be available on linux. Note: glibc does not currently provide a wrapper for this syscall yet. As a result, it is recommended to use `syscall` directly. See the man pages for details.


        Add CI build with memfd feature

Update the travis CI config to include a build forcing the memfd feature to be used. Switching to an explicit build matrix was required as the memfd feature is not used for any target OSes other than linux.

dlrobertson · 2017-02-23T23:48:36Z

The commit message for the last commit needs updating.

Good catch.

It's the default implementation, used in all cases unless we have a more specific one

Right, my bad... I didn't correctly migrate the cfg in the switch from force-posix-shm to memfd. Good catch.

antrik · 2017-02-24T02:47:53Z

@bors-servo r+

bors-servo · 2017-02-24T02:47:54Z

📌 Commit 9aa837c has been approved by antrik

bors-servo · 2017-02-24T02:47:57Z

⌛ Testing commit 9aa837c with merge 144d461...


        Auto merge of #140 - danlrobertson:shmem, r=antrik

Use memfd_create and shm_open instead of mkstemp Remove the use of `mkstemp` which creates a file in `/tmp`. Instead use the following - `linux` - [`memfd_create`](http://man7.org/linux/man-pages/man2/memfd_create.2.html) - `freebsd` - [`shm_open`](https://www.freebsd.org/cgi/man.cgi?query=shm_open&sektion=2)

bors-servo · 2017-02-24T04:22:01Z

☀️ Test successful - status-appveyor, status-travis
Approved by: antrik
Pushing 144d461 to master...

highfive assigned emilio Feb 9, 2017

highfive added the S-awaiting-review label Feb 9, 2017

dlrobertson reviewed Feb 9, 2017

View changes

antrik reviewed Feb 9, 2017

View changes

dlrobertson force-pushed the dlrobertson:shmem branch from 1d6a99f to 9ca6d95 Feb 10, 2017

dlrobertson force-pushed the dlrobertson:shmem branch from 9ca6d95 to 342df92 Feb 10, 2017

antrik reviewed Feb 10, 2017

View changes

dlrobertson changed the title ~~[WIP] Use memfd_create, shm_open, and ashmem_create_region instead of mkstemp~~ Use memfd_create, shm_open, and ashmem_create_region instead of mkstemp Feb 11, 2017

dlrobertson force-pushed the dlrobertson:shmem branch 2 times, most recently from 012bf63 to 3fd57cc Feb 11, 2017

dlrobertson reviewed Feb 11, 2017

View changes

dlrobertson force-pushed the dlrobertson:shmem branch from 3fd57cc to 37decb6 Feb 11, 2017

antrik reviewed Feb 12, 2017

View changes

dlrobertson force-pushed the dlrobertson:shmem branch from 37decb6 to 7f5664b Feb 13, 2017

dlrobertson mentioned this pull request Feb 13, 2017

Make android OSes use the unix module instead of inprocess #143

Open

dlrobertson changed the title ~~Use memfd_create, shm_open, and ashmem_create_region instead of mkstemp~~ Use memfd_create and shm_open instead of mkstemp Feb 13, 2017

dlrobertson reviewed Feb 13, 2017

View changes

dlrobertson force-pushed the dlrobertson:shmem branch 3 times, most recently from 3a3d6f6 to cf33cec Feb 13, 2017

antrik reviewed Feb 14, 2017

View changes

dlrobertson force-pushed the dlrobertson:shmem branch from 39b3a7a to 348e3a4 Feb 20, 2017

dlrobertson added 2 commits Feb 8, 2017

dlrobertson force-pushed the dlrobertson:shmem branch from 348e3a4 to e120320 Feb 20, 2017

antrik reviewed Feb 21, 2017

View changes

dlrobertson added 2 commits Feb 12, 2017

dlrobertson force-pushed the dlrobertson:shmem branch from e120320 to 9aa837c Feb 23, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Feb 24, 2017

bors-servo merged commit 9aa837c into servo:master Feb 24, 2017
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Feb 24, 2017

dlrobertson mentioned this pull request Feb 25, 2017

Use memfd for IPCSharedMemory on Linux #125

Closed

dlrobertson deleted the dlrobertson:shmem branch Feb 25, 2017

christophermaier mentioned this pull request Jul 20, 2018

Remove rust-ipc-socket files when appropriate habitat-sh/habitat#5365

Merged

servo / ipc-channel

Join GitHub today

Use memfd_create and shm_open instead of mkstemp #140

Use memfd_create and shm_open instead of mkstemp #140

Conversation

dlrobertson commented Feb 9, 2017 • edited

highfive commented Feb 9, 2017

This comment has been minimized.

This comment has been minimized.

Manishearth commented Feb 9, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

antrik commented Feb 9, 2017

Manishearth commented Feb 9, 2017

antrik left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

antrik commented Feb 9, 2017

dlrobertson commented Feb 10, 2017

antrik left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dlrobertson commented Feb 11, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dlrobertson commented Feb 18, 2017

dlrobertson commented Feb 18, 2017

antrik commented Feb 19, 2017

antrik commented Feb 19, 2017

dlrobertson commented Feb 19, 2017 • edited

dlrobertson commented Feb 9, 2017 •

edited

dlrobertson commented Feb 19, 2017 •

edited