Skip to content

Commit

Permalink
std: Ensure fs::{DirEntry, ReadDir} are Send/Sync
Browse files Browse the repository at this point in the history
The windows/unix modules were currently inconsistent about the traits being
implemented for `DirEntry` and there isn't much particular reason why the traits
*couldn't* be implemented for `ReadDir` and `DirEntry`, so this commit ensures
that they are implemented.

Closes #22577
  • Loading branch information
alexcrichton committed Feb 20, 2015
1 parent 522d09d commit 756210a
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 25 deletions.
33 changes: 20 additions & 13 deletions src/libstd/sys/unix/fs2.rs
Expand Up @@ -18,7 +18,7 @@ use libc::{self, c_int, c_void, size_t, off_t, c_char, mode_t};
use mem;
use path::{Path, PathBuf};
use ptr;
use rc::Rc;
use sync::Arc;
use sys::fd::FileDesc;
use sys::{c, cvt, cvt_r};
use sys_common::FromInner;
Expand All @@ -31,14 +31,18 @@ pub struct FileAttr {
}

pub struct ReadDir {
dirp: *mut libc::DIR,
root: Rc<PathBuf>,
dirp: Dir,
root: Arc<PathBuf>,
}

struct Dir(*mut libc::DIR);

unsafe impl Send for Dir {}
unsafe impl Sync for Dir {}

pub struct DirEntry {
buf: Vec<u8>,
dirent: *mut libc::dirent_t,
root: Rc<PathBuf>,
buf: Vec<u8>, // actually *mut libc::dirent_t
root: Arc<PathBuf>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -109,7 +113,7 @@ impl Iterator for ReadDir {

let mut entry_ptr = ptr::null_mut();
loop {
if unsafe { libc::readdir_r(self.dirp, ptr, &mut entry_ptr) != 0 } {
if unsafe { libc::readdir_r(self.dirp.0, ptr, &mut entry_ptr) != 0 } {
return Some(Err(Error::last_os_error()))
}
if entry_ptr.is_null() {
Expand All @@ -118,7 +122,6 @@ impl Iterator for ReadDir {

let entry = DirEntry {
buf: buf,
dirent: entry_ptr,
root: self.root.clone()
};
if entry.name_bytes() == b"." || entry.name_bytes() == b".." {
Expand All @@ -130,9 +133,9 @@ impl Iterator for ReadDir {
}
}

impl Drop for ReadDir {
impl Drop for Dir {
fn drop(&mut self) {
let r = unsafe { libc::closedir(self.dirp) };
let r = unsafe { libc::closedir(self.0) };
debug_assert_eq!(r, 0);
}
}
Expand All @@ -147,9 +150,13 @@ impl DirEntry {
fn rust_list_dir_val(ptr: *mut libc::dirent_t) -> *const c_char;
}
unsafe {
CStr::from_ptr(rust_list_dir_val(self.dirent)).to_bytes()
CStr::from_ptr(rust_list_dir_val(self.dirent())).to_bytes()
}
}

fn dirent(&self) -> *mut libc::dirent_t {
self.buf.as_ptr() as *mut _
}
}

impl OpenOptions {
Expand Down Expand Up @@ -279,14 +286,14 @@ pub fn mkdir(p: &Path) -> io::Result<()> {
}

pub fn readdir(p: &Path) -> io::Result<ReadDir> {
let root = Rc::new(p.to_path_buf());
let root = Arc::new(p.to_path_buf());
let p = try!(cstr(p));
unsafe {
let ptr = libc::opendir(p.as_ptr());
if ptr.is_null() {
Err(Error::last_os_error())
} else {
Ok(ReadDir { dirp: ptr, root: root })
Ok(ReadDir { dirp: Dir(ptr), root: root })
}
}
}
Expand Down
39 changes: 27 additions & 12 deletions src/libstd/sys/windows/fs2.rs
Expand Up @@ -19,6 +19,7 @@ use libc::{self, HANDLE};
use mem;
use path::{Path, PathBuf};
use ptr;
use sync::Arc;
use sys::handle::Handle as RawHandle;
use sys::{c, cvt};
use vec::Vec;
Expand All @@ -27,12 +28,20 @@ pub struct File { handle: RawHandle }
pub struct FileAttr { data: c::WIN32_FILE_ATTRIBUTE_DATA }

pub struct ReadDir {
handle: libc::HANDLE,
root: PathBuf,
handle: FindNextFileHandle,
root: Arc<PathBuf>,
first: Option<libc::WIN32_FIND_DATAW>,
}

pub struct DirEntry { path: PathBuf }
struct FindNextFileHandle(libc::HANDLE);

unsafe impl Send for FindNextFileHandle {}
unsafe impl Sync for FindNextFileHandle {}

pub struct DirEntry {
root: Arc<PathBuf>,
data: libc::WIN32_FIND_DATAW,
}

#[derive(Clone, Default)]
pub struct OpenOptions {
Expand Down Expand Up @@ -61,7 +70,7 @@ impl Iterator for ReadDir {
unsafe {
let mut wfd = mem::zeroed();
loop {
if libc::FindNextFileW(self.handle, &mut wfd) == 0 {
if libc::FindNextFileW(self.handle.0, &mut wfd) == 0 {
if libc::GetLastError() ==
c::ERROR_NO_MORE_FILES as libc::DWORD {
return None
Expand All @@ -77,29 +86,31 @@ impl Iterator for ReadDir {
}
}

impl Drop for ReadDir {
impl Drop for FindNextFileHandle {
fn drop(&mut self) {
let r = unsafe { libc::FindClose(self.handle) };
let r = unsafe { libc::FindClose(self.0) };
debug_assert!(r != 0);
}
}

impl DirEntry {
fn new(root: &Path, wfd: &libc::WIN32_FIND_DATAW) -> Option<DirEntry> {
fn new(root: &Arc<PathBuf>, wfd: &libc::WIN32_FIND_DATAW) -> Option<DirEntry> {
match &wfd.cFileName[0..3] {
// check for '.' and '..'
[46, 0, ..] |
[46, 46, 0, ..] => return None,
_ => {}
}

let filename = super::truncate_utf16_at_nul(&wfd.cFileName);
let filename: OsString = OsStringExt::from_wide(filename);
Some(DirEntry { path: root.join(&filename) })
Some(DirEntry {
root: root.clone(),
data: *wfd,
})
}

pub fn path(&self) -> PathBuf {
self.path.clone()
let filename = super::truncate_utf16_at_nul(&self.data.cFileName);
self.root.join(&<OsString as OsStringExt>::from_wide(filename))
}
}

Expand Down Expand Up @@ -312,7 +323,11 @@ pub fn readdir(p: &Path) -> io::Result<ReadDir> {
let mut wfd = mem::zeroed();
let find_handle = libc::FindFirstFileW(path.as_ptr(), &mut wfd);
if find_handle != libc::INVALID_HANDLE_VALUE {
Ok(ReadDir { handle: find_handle, root: root, first: Some(wfd) })
Ok(ReadDir {
handle: FindNextFileHandle(find_handle),
root: Arc::new(root),
first: Some(wfd),
})
} else {
Err(Error::last_os_error())
}
Expand Down
29 changes: 29 additions & 0 deletions src/test/run-pass/issue-22577.rs
@@ -0,0 +1,29 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::{fs, net};

fn assert_both<T: Send + Sync>() {}

fn main() {
assert_both::<fs::File>();
assert_both::<fs::Metadata>();
assert_both::<fs::ReadDir>();
assert_both::<fs::DirEntry>();
assert_both::<fs::WalkDir>();
assert_both::<fs::OpenOptions>();
assert_both::<fs::Permissions>();

assert_both::<net::TcpStream>();
assert_both::<net::TcpListener>();
assert_both::<net::UdpSocket>();
assert_both::<net::SocketAddr>();
assert_both::<net::IpAddr>();
}

0 comments on commit 756210a

Please sign in to comment.