From 261ad31b9ad58060189fc9f7121c7d2422c964b9 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 19 Jun 2018 07:44:01 +0600 Subject: [PATCH] remove some unsafe code --- src/pipeline.rs | 4 +-- src/server/channel.rs | 1 - src/server/encoding.rs | 2 -- src/server/h1writer.rs | 60 ++++++++++++++++++++++-------------------- src/server/h2writer.rs | 4 +-- src/server/helpers.rs | 18 ++++++------- src/server/mod.rs | 5 ++-- src/server/shared.rs | 45 +++++++++++-------------------- src/server/srv.rs | 10 +++---- src/server/worker.rs | 2 -- 10 files changed, 64 insertions(+), 87 deletions(-) diff --git a/src/pipeline.rs b/src/pipeline.rs index d08a739dd..91e2143ea 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -547,7 +547,7 @@ impl ProcessResponse { } Ok(Async::Ready(Some(chunk))) => { self.iostate = IOState::Payload(body); - match io.write(chunk.into()) { + match io.write(&chunk.into()) { Err(err) => { info.error = Some(err.into()); return Ok(FinishingMiddlewares::init( @@ -592,7 +592,7 @@ impl ProcessResponse { break 'inner; } Frame::Chunk(Some(chunk)) => { - match io.write(chunk) { + match io.write(&chunk) { Err(err) => { info.error = Some(err.into()); return Ok( diff --git a/src/server/channel.rs b/src/server/channel.rs index a38ecc936..d236963b5 100644 --- a/src/server/channel.rs +++ b/src/server/channel.rs @@ -203,7 +203,6 @@ impl Node { } fn insert(&self, next: &Node) { - #[allow(mutable_transmutes)] unsafe { if let Some(ref next2) = self.next { let n: &mut Node<()> = diff --git a/src/server/encoding.rs b/src/server/encoding.rs index 6d814482e..db0fd0c37 100644 --- a/src/server/encoding.rs +++ b/src/server/encoding.rs @@ -701,8 +701,6 @@ pub(crate) struct TransferEncoding { kind: TransferEncodingKind, } -unsafe impl Send for TransferEncoding {} - #[derive(Debug, PartialEq, Clone)] enum TransferEncodingKind { /// An Encoder for when Transfer-Encoding includes `chunked`. diff --git a/src/server/h1writer.rs b/src/server/h1writer.rs index a144a2ff9..01477464b 100644 --- a/src/server/h1writer.rs +++ b/src/server/h1writer.rs @@ -106,7 +106,7 @@ impl Writer for H1Writer { } #[inline] - fn buffer(&self) -> &mut BytesMut { + fn buffer(&mut self) -> &mut BytesMut { self.buffer.get_mut() } @@ -181,35 +181,37 @@ impl Writer for H1Writer { let mut pos = 0; let mut has_date = false; let mut remaining = buffer.remaining_mut(); - let mut buf = unsafe { &mut *(buffer.bytes_mut() as *mut [u8]) }; - for (key, value) in msg.headers() { - if is_bin && key == CONTENT_LENGTH { - is_bin = false; - continue; - } - has_date = has_date || key == DATE; - let v = value.as_ref(); - let k = key.as_str().as_bytes(); - let len = k.len() + v.len() + 4; - if len > remaining { - unsafe { buffer.advance_mut(pos) }; - pos = 0; - buffer.reserve(len); - remaining = buffer.remaining_mut(); - buf = unsafe { &mut *(buffer.bytes_mut() as *mut _) }; - } + unsafe { + let mut buf = &mut *(buffer.bytes_mut() as *mut [u8]); + for (key, value) in msg.headers() { + if is_bin && key == CONTENT_LENGTH { + is_bin = false; + continue; + } + has_date = has_date || key == DATE; + let v = value.as_ref(); + let k = key.as_str().as_bytes(); + let len = k.len() + v.len() + 4; + if len > remaining { + buffer.advance_mut(pos); + pos = 0; + buffer.reserve(len); + remaining = buffer.remaining_mut(); + buf = &mut *(buffer.bytes_mut() as *mut _); + } - buf[pos..pos + k.len()].copy_from_slice(k); - pos += k.len(); - buf[pos..pos + 2].copy_from_slice(b": "); - pos += 2; - buf[pos..pos + v.len()].copy_from_slice(v); - pos += v.len(); - buf[pos..pos + 2].copy_from_slice(b"\r\n"); - pos += 2; - remaining -= len; + buf[pos..pos + k.len()].copy_from_slice(k); + pos += k.len(); + buf[pos..pos + 2].copy_from_slice(b": "); + pos += 2; + buf[pos..pos + v.len()].copy_from_slice(v); + pos += v.len(); + buf[pos..pos + 2].copy_from_slice(b"\r\n"); + pos += 2; + remaining -= len; + } + buffer.advance_mut(pos); } - unsafe { buffer.advance_mut(pos) }; // optimized date header, set_date writes \r\n if !has_date { @@ -233,7 +235,7 @@ impl Writer for H1Writer { Ok(WriterState::Done) } - fn write(&mut self, payload: Binary) -> io::Result { + fn write(&mut self, payload: &Binary) -> io::Result { self.written += payload.len() as u64; if !self.flags.contains(Flags::DISCONNECTED) { if self.flags.contains(Flags::STARTED) { diff --git a/src/server/h2writer.rs b/src/server/h2writer.rs index f019f75b3..c2731b112 100644 --- a/src/server/h2writer.rs +++ b/src/server/h2writer.rs @@ -77,7 +77,7 @@ impl Writer for H2Writer { } #[inline] - fn buffer(&self) -> &mut BytesMut { + fn buffer(&mut self) -> &mut BytesMut { self.buffer.get_mut() } @@ -164,7 +164,7 @@ impl Writer for H2Writer { } } - fn write(&mut self, payload: Binary) -> io::Result { + fn write(&mut self, payload: &Binary) -> io::Result { self.written = payload.len() as u64; if !self.flags.contains(Flags::DISCONNECTED) { diff --git a/src/server/helpers.rs b/src/server/helpers.rs index cf497edd7..939785f4c 100644 --- a/src/server/helpers.rs +++ b/src/server/helpers.rs @@ -7,7 +7,7 @@ use std::{mem, ptr, slice}; use httprequest::HttpInnerMessage; -/// Internal use only! unsafe +/// Internal use only! pub(crate) struct SharedMessagePool(RefCell>>); impl SharedMessagePool { @@ -189,14 +189,14 @@ pub fn write_content_length(mut n: usize, bytes: &mut BytesMut) { } pub(crate) fn convert_usize(mut n: usize, bytes: &mut BytesMut) { - let mut curr: isize = 39; - let mut buf: [u8; 41] = unsafe { mem::uninitialized() }; - buf[39] = b'\r'; - buf[40] = b'\n'; - let buf_ptr = buf.as_mut_ptr(); - let lut_ptr = DEC_DIGITS_LUT.as_ptr(); - unsafe { + let mut curr: isize = 39; + let mut buf: [u8; 41] = mem::uninitialized(); + buf[39] = b'\r'; + buf[40] = b'\n'; + let buf_ptr = buf.as_mut_ptr(); + let lut_ptr = DEC_DIGITS_LUT.as_ptr(); + // eagerly decode 4 characters at a time while n >= 10_000 { let rem = (n % 10_000) as isize; @@ -229,9 +229,7 @@ pub(crate) fn convert_usize(mut n: usize, bytes: &mut BytesMut) { curr -= 2; ptr::copy_nonoverlapping(lut_ptr.offset(d1), buf_ptr.offset(curr), 2); } - } - unsafe { bytes.extend_from_slice(slice::from_raw_parts( buf_ptr.offset(curr), 41 - curr as usize, diff --git a/src/server/mod.rs b/src/server/mod.rs index 91b4d1fa9..c0dabb263 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -191,15 +191,14 @@ pub trait Writer { fn set_date(&self, st: &mut BytesMut); #[doc(hidden)] - #[cfg_attr(feature = "cargo-clippy", allow(mut_from_ref))] - fn buffer(&self) -> &mut BytesMut; + fn buffer(&mut self) -> &mut BytesMut; fn start( &mut self, req: &mut HttpInnerMessage, resp: &mut HttpResponse, encoding: ContentEncoding, ) -> io::Result; - fn write(&mut self, payload: Binary) -> io::Result; + fn write(&mut self, payload: &Binary) -> io::Result; fn write_eof(&mut self) -> io::Result; diff --git a/src/server/shared.rs b/src/server/shared.rs index 54f7b1e65..a36c46176 100644 --- a/src/server/shared.rs +++ b/src/server/shared.rs @@ -6,58 +6,52 @@ use std::rc::Rc; use body::Binary; -/// Internal use only! unsafe #[derive(Debug)] -pub(crate) struct SharedBytesPool(RefCell>>); +pub(crate) struct SharedBytesPool(RefCell>); impl SharedBytesPool { pub fn new() -> SharedBytesPool { SharedBytesPool(RefCell::new(VecDeque::with_capacity(128))) } - pub fn get_bytes(&self) -> Rc { + pub fn get_bytes(&self) -> BytesMut { if let Some(bytes) = self.0.borrow_mut().pop_front() { bytes } else { - Rc::new(BytesMut::new()) + BytesMut::new() } } - pub fn release_bytes(&self, mut bytes: Rc) { + pub fn release_bytes(&self, mut bytes: BytesMut) { let v = &mut self.0.borrow_mut(); if v.len() < 128 { - Rc::get_mut(&mut bytes).unwrap().clear(); + bytes.clear(); v.push_front(bytes); } } } #[derive(Debug)] -pub(crate) struct SharedBytes(Option>, Option>); +pub(crate) struct SharedBytes(Option, Option>); impl Drop for SharedBytes { fn drop(&mut self) { if let Some(pool) = self.1.take() { if let Some(bytes) = self.0.take() { - if Rc::strong_count(&bytes) == 1 { - pool.release_bytes(bytes); - } + pool.release_bytes(bytes); } } } } impl SharedBytes { - pub fn new(bytes: Rc, pool: Rc) -> SharedBytes { + pub fn new(bytes: BytesMut, pool: Rc) -> SharedBytes { SharedBytes(Some(bytes), Some(pool)) } - #[inline(always)] - #[allow(mutable_transmutes)] - #[cfg_attr(feature = "cargo-clippy", allow(mut_from_ref, inline_always))] - pub(crate) fn get_mut(&self) -> &mut BytesMut { - let r: &BytesMut = self.0.as_ref().unwrap().as_ref(); - unsafe { &mut *(r as *const _ as *mut _) } + #[inline] + pub(crate) fn get_mut(&mut self) -> &mut BytesMut { + self.0.as_mut().unwrap() } #[inline] @@ -75,17 +69,16 @@ impl SharedBytes { self.0.as_ref().unwrap().as_ref() } - pub fn split_to(&self, n: usize) -> BytesMut { + pub fn split_to(&mut self, n: usize) -> BytesMut { self.get_mut().split_to(n) } - pub fn take(&self) -> BytesMut { + pub fn take(&mut self) -> BytesMut { self.get_mut().take() } #[inline] - #[cfg_attr(feature = "cargo-clippy", allow(needless_pass_by_value))] - pub fn extend(&self, data: Binary) { + pub fn extend(&mut self, data: &Binary) { let buf = self.get_mut(); let data = data.as_ref(); buf.reserve(data.len()); @@ -93,7 +86,7 @@ impl SharedBytes { } #[inline] - pub fn extend_from_slice(&self, data: &[u8]) { + pub fn extend_from_slice(&mut self, data: &[u8]) { let buf = self.get_mut(); buf.reserve(data.len()); SharedBytes::put_slice(buf, data); @@ -117,13 +110,7 @@ impl SharedBytes { impl Default for SharedBytes { fn default() -> Self { - SharedBytes(Some(Rc::new(BytesMut::new())), None) - } -} - -impl Clone for SharedBytes { - fn clone(&self) -> SharedBytes { - SharedBytes(self.0.clone(), self.1.clone()) + SharedBytes(Some(BytesMut::new()), None) } } diff --git a/src/server/srv.rs b/src/server/srv.rs index 89f57b891..cd6703663 100644 --- a/src/server/srv.rs +++ b/src/server/srv.rs @@ -64,8 +64,6 @@ where no_signals: bool, } -unsafe impl Send for HttpServer {} - enum ServerCommand { WorkerDied(usize, Slab), } @@ -485,11 +483,9 @@ impl HttpServer { self.exit = true; self.no_signals = false; - let _ = thread::spawn(move || { - let sys = System::new("http-server"); - self.start(); - sys.run(); - }).join(); + let sys = System::new("http-server"); + self.start(); + sys.run(); } } diff --git a/src/server/worker.rs b/src/server/worker.rs index 6cf2bbd68..3d4ee8633 100644 --- a/src/server/worker.rs +++ b/src/server/worker.rs @@ -65,8 +65,6 @@ where tcp_ka: Option, } -unsafe impl Send for Worker {} - impl Worker { pub(crate) fn new( h: Vec, socks: Slab, keep_alive: KeepAlive,