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

Closure transfer fix for sched::clone #920

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b5ef299
Closure transfer fix for `sched::clone`
xurtis Jul 4, 2018
45ccdfe
Revert "Closure transfer fix for `sched::clone`"
xurtis Jul 5, 2018
f3fc220
Simply forget the boxed function.
xurtis Jul 5, 2018
28aa11d
Function should be safe to send.
xurtis Jul 5, 2018
085b147
Fix type annotation.
xurtis Jul 5, 2018
0e5a6e6
Remove masking of erroneous cast of function arguments.
xurtis Jul 6, 2018
9979947
Fix boxing of closure before sending.
xurtis Jul 6, 2018
dd2e37f
Add tests for clone.
xurtis Jul 6, 2018
245d898
Use dynamically allocated stack in test.
xurtis Jul 6, 2018
eaaf6e7
Simpler return from clone.
xurtis Jul 6, 2018
761c035
Fix test to work on 1.20
xurtis Jul 6, 2018
e78c659
Mask clone test.
xurtis Jul 6, 2018
f6f9a29
Add test output
xurtis Jul 7, 2018
0e1a7da
Revert "Mask clone test."
xurtis Jul 7, 2018
69569e2
Simplify stack allocation.
xurtis Jul 7, 2018
d1ae099
Add changelog.
xurtis Jul 7, 2018
d9588a2
Add backtrace to test output.
xurtis Jul 7, 2018
7c46462
Don't use child signal to wait for clone test.
xurtis Jul 7, 2018
d0499dd
Remove backtrace flag.
xurtis Jul 7, 2018
1db50bd
Don't clone VM for test.
xurtis Jul 7, 2018
efacf5f
Upgrade travis to xenial.
xurtis Jul 7, 2018
7bef4b9
Revert "Upgrade travis to xenial."
xurtis Jul 7, 2018
7c313ab
Add error hints to conflicting signal tests.
xurtis Jul 7, 2018
832a5ec
Use `isize::wrapping_neg` for pointer alignment.
xurtis Jul 7, 2018
8701c5c
Move changelog statement.
xurtis Jul 8, 2018
2c099e1
Nocapture unnecessary.
xurtis Jul 8, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ env:
global:
- CRATE_NAME=nix
- RUST_TEST_THREADS=1
- RUST_TEST_NOCAPTURE=1

matrix:
# These are all the build jobs. Adjust as necessary. Comment out what you
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## [0.11.0] 2018-06-01

### Added
- Changed mechanism used to send closure to child thread in `sched::clone`.
Copy link
Member

Choose a reason for hiding this comment

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

This description doesn't say why you "changed mechanism". Did you fix a bug? Add a feature? Change just for the heck of it? Also, it's in the wrong section. "Added" is for new features. "Fixed" is for bug fixes. "Changed" is for backwards-incompatible changes, including bug fixes.

([#920](https://github.com/nix-rust/nix/pull/920))
- Added `sendfile` on FreeBSD and Darwin.
([#901](https://github.com/nix-rust/nix/pull/901))
- Added `pselect`
Expand Down
45 changes: 37 additions & 8 deletions src/sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ libc_bitflags!{
}
}

pub type CloneCb<'a> = Box<FnMut() -> isize + 'a>;
pub type CloneCb = Box<FnMut() -> isize + Send + 'static>;

#[repr(C)]
#[derive(Clone, Copy)]
Expand Down Expand Up @@ -85,24 +85,24 @@ pub fn sched_setaffinity(pid: Pid, cpuset: &CpuSet) -> Result<()> {
Errno::result(res).map(drop)
}

pub fn clone(mut cb: CloneCb,
pub fn clone(cb: CloneCb,
stack: &mut [u8],
flags: CloneFlags,
signal: Option<c_int>)
-> Result<Pid> {
extern "C" fn callback(data: *mut CloneCb) -> c_int {
let cb: &mut CloneCb = unsafe { &mut *data };
(*cb)() as c_int
extern "C" fn callback(data: *mut c_void) -> c_int {
let mut cb: CloneCb = unsafe { *Box::from_raw(data as *mut CloneCb) };
cb() as c_int
}

let res = unsafe {
let combined = flags.bits() | signal.unwrap_or(0);
let ptr = stack.as_mut_ptr().offset(stack.len() as isize);
let ptr_aligned = ptr.offset((ptr as usize % 16) as isize * -1);
libc::clone(mem::transmute(callback as extern "C" fn(*mut Box<::std::ops::FnMut() -> isize>) -> i32),
let ptr_aligned = ptr.offset(((ptr as usize % 16) as isize).wrapping_neg());
libc::clone(callback,
ptr_aligned as *mut c_void,
combined,
&mut cb as *mut _ as *mut c_void)
Box::into_raw(Box::new(cb)) as *mut c_void)
};

Errno::result(res).map(Pid::from_raw)
Expand All @@ -119,3 +119,32 @@ pub fn setns(fd: RawFd, nstype: CloneFlags) -> Result<()> {

Errno::result(res).map(drop)
}

#[cfg(test)]
mod test {
use super::*;
use sys::wait::{waitpid, WaitStatus, WaitPidFlag};

fn clone_payload() -> Box<FnMut() -> isize + Send + 'static> {
let numbers: Vec<i32> = (0..101).into_iter().collect();
Box::new(move || {
assert_eq!(numbers.iter().sum::<i32>(), 5050);
0
})
}

#[test]
fn simple_clone() {
// Stack *must* outlive the child.
let mut stack = vec![0u8; 4096];
let pid = clone(
clone_payload(),
stack.as_mut(),
CloneFlags::empty(),
None,
).expect("Executing child");

let exit_status = waitpid(pid, Some(WaitPidFlag::__WALL)).expect("Waiting for child");
assert_eq!(exit_status, WaitStatus::Exited(pid, 0));
}
}
6 changes: 3 additions & 3 deletions src/sys/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,9 +800,9 @@ mod tests {
let mut mask = SigSet::empty();
mask.add(SIGUSR1);
mask.add(SIGUSR2);
mask.thread_block().unwrap();
mask.thread_block().expect("Setting thread block signals");

raise(SIGUSR1).unwrap();
assert_eq!(mask.wait().unwrap(), SIGUSR1);
raise(SIGUSR1).expect("Raising SIGUSR1");
assert_eq!(mask.wait().expect("Waiting on signal"), SIGUSR1);
}
}