Skip to content

Commit

Permalink
Merge #907
Browse files Browse the repository at this point in the history 
907: Remove emulation of FD_CLOEXEC/O_NONBLOCK r=asomers a=Susurrus

Rather than using the native implementation of these constants
on supported platforms, the native implementation was instead
emulated. This was also hidden from the user even though this
could result in data races and the functionality being broken.

Native functionality is, however, not support on macos/ios.
Rather than enable this emulation solely for this platform, it
should be removed as this is a dangerous abstraction.

This is a replacement for #863. There is much previous discussion there which I recommend you read to familiarize yourself with this decision.

I'm looking to push this through rather quickly as it's the last thing blocking our next 0.11.0 release.

cc @aomser @kristate

Co-authored-by: Bryant Mairs <bryantmairs@google.com>
  • Loading branch information
@bors
bors[bot] and Bryant Mairs committed Jun 2, 2018
2 parents badb451 + 7e870c0 commit 72cebbc
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 98 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#892](https://github.com/nix-rust/nix/pull/892))
- Remove `IFF_NOTRAILERS` on OpenBSD, as it has been removed in OpenBSD 6.3
([#893](https://github.com/nix-rust/nix/pull/893))
- Emulation of `FD_CLOEXEC` and `O_NONBLOCK` was removed from `socket()`, `accept4()`, and
`socketpair()`.
([#907](https://github.com/nix-rust/nix/pull/907))

### Fixed
- Fixed possible panics when using `SigAction::flags` on Linux
Expand All @@ -68,6 +71,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#872](https://github.com/nix-rust/nix/pull/872))
- Removed `sys::aio::lio_listio`. Use `sys::aio::LioCb::listio` instead.
([#872](https://github.com/nix-rust/nix/pull/872))
- Removed emulated `accept4()` from macos, ios, and netbsd targets
([#907](https://github.com/nix-rust/nix/pull/907))

## [0.10.0] 2018-01-26

Expand Down
119 changes: 21 additions & 98 deletions src/sys/socket/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//! [Further reading](http://man7.org/linux/man-pages/man7/socket.7.html)
use {Error, Result};
use errno::Errno;
use features;
use libc::{self, c_void, c_int, socklen_t, size_t};
use std::{fmt, mem, ptr, slice};
use std::os::unix::io::RawFd;
Expand Down Expand Up @@ -692,87 +691,43 @@ pub fn recvmsg<'a, T>(fd: RawFd, iov: &[IoVec<&mut [u8]>], cmsg_buffer: Option<&
///
/// [Further reading](http://pubs.opengroup.org/onlinepubs/9699919799/functions/socket.html)
pub fn socket<T: Into<Option<SockProtocol>>>(domain: AddressFamily, ty: SockType, flags: SockFlag, protocol: T) -> Result<RawFd> {
let mut ty = ty as c_int;
let protocol = match protocol.into() {
None => 0,
Some(p) => p as c_int,
};
let feat_atomic = features::socket_atomic_cloexec();

if feat_atomic {
ty |= flags.bits();
}

// TODO: Check the kernel version
let res = try!(Errno::result(unsafe { libc::socket(domain as c_int, ty, protocol) }));

#[cfg(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd"))]
{
use fcntl::{fcntl, FdFlag, OFlag};
use fcntl::FcntlArg::{F_SETFD, F_SETFL};

if !feat_atomic {
if flags.contains(SockFlag::SOCK_CLOEXEC) {
try!(fcntl(res, F_SETFD(FdFlag::FD_CLOEXEC)));
}
// SockFlags are usually embedded into `ty`, but we don't do that in `nix` because it's a
// little easier to understand by separating it out. So we have to merge these bitfields
// here.
let mut ty = ty as c_int;
ty |= flags.bits();

if flags.contains(SockFlag::SOCK_NONBLOCK) {
try!(fcntl(res, F_SETFL(OFlag::O_NONBLOCK)));
}
}
}
let res = unsafe { libc::socket(domain as c_int, ty, protocol) };

Ok(res)
Errno::result(res)
}

/// Create a pair of connected sockets
///
/// [Further reading](http://pubs.opengroup.org/onlinepubs/9699919799/functions/socketpair.html)
pub fn socketpair<T: Into<Option<SockProtocol>>>(domain: AddressFamily, ty: SockType, protocol: T,
flags: SockFlag) -> Result<(RawFd, RawFd)> {
let mut ty = ty as c_int;
let protocol = match protocol.into() {
None => 0,
Some(p) => p as c_int,
};
let feat_atomic = features::socket_atomic_cloexec();

if feat_atomic {
ty |= flags.bits();
}
// SockFlags are usually embedded into `ty`, but we don't do that in `nix` because it's a
// little easier to understand by separating it out. So we have to merge these bitfields
// here.
let mut ty = ty as c_int;
ty |= flags.bits();

let mut fds = [-1, -1];
let res = unsafe {
libc::socketpair(domain as c_int, ty, protocol, fds.as_mut_ptr())
};
try!(Errno::result(res));

#[cfg(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd"))]
{
use fcntl::{fcntl, FdFlag, OFlag};
use fcntl::FcntlArg::{F_SETFD, F_SETFL};

if !feat_atomic {
if flags.contains(SockFlag::SOCK_CLOEXEC) {
try!(fcntl(fds[0], F_SETFD(FdFlag::FD_CLOEXEC)));
try!(fcntl(fds[1], F_SETFD(FdFlag::FD_CLOEXEC)));
}
let res = unsafe { libc::socketpair(domain as c_int, ty, protocol, fds.as_mut_ptr()) };
Errno::result(res)?;

if flags.contains(SockFlag::SOCK_NONBLOCK) {
try!(fcntl(fds[0], F_SETFL(OFlag::O_NONBLOCK)));
try!(fcntl(fds[1], F_SETFL(OFlag::O_NONBLOCK)));
}
}
}
Ok((fds[0], fds[1]))
}

Expand Down Expand Up @@ -809,46 +764,14 @@ pub fn accept(sockfd: RawFd) -> Result<RawFd> {
/// Accept a connection on a socket
///
/// [Further reading](http://man7.org/linux/man-pages/man2/accept.2.html)
#[cfg(any(target_os = "android",
target_os = "freebsd",
target_os = "linux",
target_os = "openbsd"))]
pub fn accept4(sockfd: RawFd, flags: SockFlag) -> Result<RawFd> {
accept4_polyfill(sockfd, flags)
}

#[inline]
fn accept4_polyfill(sockfd: RawFd, flags: SockFlag) -> Result<RawFd> {
let res = try!(Errno::result(unsafe { libc::accept(sockfd, ptr::null_mut(), ptr::null_mut()) }));

#[cfg(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd"))]
{
use fcntl::{fcntl, FdFlag, OFlag};
use fcntl::FcntlArg::{F_SETFD, F_SETFL};

if flags.contains(SockFlag::SOCK_CLOEXEC) {
try!(fcntl(res, F_SETFD(FdFlag::FD_CLOEXEC)));
}

if flags.contains(SockFlag::SOCK_NONBLOCK) {
try!(fcntl(res, F_SETFL(OFlag::O_NONBLOCK)));
}
}

// Disable unused variable warning on some platforms
#[cfg(not(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd")))]
{
let _ = flags;
}
let res = unsafe { libc::accept4(sockfd, ptr::null_mut(), ptr::null_mut(), flags.bits()) };


Ok(res)
Errno::result(res)
}

/// Initiate a connection on a socket
Expand Down
62 changes: 62 additions & 0 deletions test/sys/test_socket.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,65 @@ pub fn test_syscontrol() {
// requires root privileges
// connect(fd, &sockaddr).expect("connect failed");
}

/// Test non-blocking mode on new sockets via SockFlag::O_NONBLOCK
#[cfg(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd"))]
#[test]
pub fn test_sockflag_nonblock() {
use libc;
use nix::fcntl::{fcntl};
use nix::fcntl::FcntlArg::{F_GETFL};
use nix::sys::socket::{socket, AddressFamily, SockType, SockFlag};

/* first, try without SockFlag::SOCK_NONBLOCK */
let sock = socket(AddressFamily::Unix, SockType::Stream, SockFlag::empty(), None)
.expect("socket failed");

let fcntl_res = fcntl(sock, F_GETFL).expect("fcntl failed");

assert!(fcntl_res & libc::O_NONBLOCK == 0);

/* next, try with SockFlag::SOCK_NONBLOCK */
let sock = socket(AddressFamily::Unix, SockType::Stream, SockFlag::SOCK_NONBLOCK, None)
.expect("socket failed");

let fcntl_res = fcntl(sock, F_GETFL).expect("fcntl failed");

assert!(fcntl_res & libc::O_NONBLOCK == libc::O_NONBLOCK);
}

/// Test close-on-exec on new sockets via SockFlag::SOCK_CLOEXEC
#[cfg(any(target_os = "android",
target_os = "dragonfly",
target_os = "freebsd",
target_os = "linux",
target_os = "netbsd",
target_os = "openbsd"))]
#[test]
pub fn test_sockflag_cloexec() {
use libc;
use nix::fcntl::{fcntl};
use nix::fcntl::FcntlArg::{F_GETFD};
use nix::sys::socket::{socket, AddressFamily, SockType, SockFlag};

/* first, test without SockFlag::SOCK_CLOEXEC */
let sock = socket(AddressFamily::Unix, SockType::Stream, SockFlag::empty(), None)
.expect("socket failed");

let fcntl_res = fcntl(sock, F_GETFD).expect("fcntl failed");

assert!(fcntl_res & libc::FD_CLOEXEC == 0);

/* next, test without SockFlag::SOCK_CLOEXEC */
let sock = socket(AddressFamily::Unix, SockType::Stream, SockFlag::SOCK_CLOEXEC, None)
.expect("socket failed");

let fcntl_res = fcntl(sock, F_GETFD).expect("fcntl failed");

assert!(fcntl_res & libc::FD_CLOEXEC == libc::FD_CLOEXEC);
}

0 comments on commit 72cebbc

Please sign in to comment.