From 4c73894c6f64ae92d799a1f7952351ef3a169eee Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Sun, 8 Mar 2020 20:44:04 +0000 Subject: [PATCH 01/11] wip: tcp changes --- cli/ops/io.rs | 85 ++++++++++++++++++++++++++++++++++++++++++++------ cli/ops/net.rs | 20 +++++++----- 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/cli/ops/io.rs b/cli/ops/io.rs index 2562b4c559682d..215f3617944f8b 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -87,6 +87,61 @@ pub struct FileMetadata { pub tty: TTYMetadata, } +pub struct StreamResourceHolder { + resource: StreamResource, + waker: Option, +} + +impl StreamResourceHolder { + pub fn new(resource: StreamResource) -> StreamResourceHolder { + StreamResourceHolder { + resource, + waker: None, + } + } +} + +impl Drop for StreamResourceHolder { + fn drop(&mut self) { + self.wake_task(); + } +} + +impl StreamResourceHolder { + /// Track the current task so future awaiting for connection + /// can be notified when listener is closed. + /// + /// Throws an error if another task is already tracked. + pub fn track_task(&mut self, cx: &Context) -> Result<(), OpError> { + // Currently, we only allow tracking a single accept task for a listener. + // This might be changed in the future with multiple workers. + // Caveat: TcpListener by itself also only tracks an accept task at a time. + // See https://github.com/tokio-rs/tokio/issues/846#issuecomment-454208883 + if self.waker.is_some() { + return Err(OpError::other("Another accept task is ongoing".to_string())); + } + let waker = futures::task::AtomicWaker::new(); + waker.register(cx.waker()); + self.waker.replace(waker); + Ok(()) + } + + /// Notifies a task when listener is closed so accept future can resolve. + pub fn wake_task(&mut self) { + if let Some(waker) = self.waker.as_ref() { + waker.wake(); + } + } + + /// Stop tracking a task. + /// Happens when the task is done and thus no further tracking is needed. + pub fn untrack_task(&mut self) { + if self.waker.is_some() { + self.waker.take(); + } + } +} + pub enum StreamResource { Stdin(tokio::io::Stdin, TTYMetadata), Stdout(tokio::fs::File), @@ -150,10 +205,22 @@ pub fn op_read( poll_fn(move |cx| { let resource_table = &mut state.borrow_mut().resource_table; - let resource = resource_table - .get_mut::(rid as u32) + let resource_holder = resource_table + .get_mut::(rid as u32) .ok_or_else(OpError::bad_resource_id)?; - let nread = ready!(resource.poll_read(cx, &mut buf.as_mut()[..]))?; + let nread = match resource_holder + .resource + .poll_read(cx, &mut buf.as_mut()[..]) + { + Poll::Ready(t) => { + resource_holder.untrack_task(); + t + } + Poll::Pending => { + resource_holder.track_task(cx)?; + return Poll::Pending; + } + }?; Poll::Ready(Ok(nread as i32)) }) .boxed_local() @@ -233,10 +300,10 @@ pub fn op_write( async move { let nwritten = poll_fn(|cx| { let resource_table = &mut state.borrow_mut().resource_table; - let resource = resource_table - .get_mut::(rid as u32) + let resource_holder = resource_table + .get_mut::(rid as u32) .ok_or_else(OpError::bad_resource_id)?; - resource.poll_write(cx, &buf.as_ref()[..]) + resource_holder.resource.poll_write(cx, &buf.as_ref()[..]) }) .await?; @@ -246,10 +313,10 @@ pub fn op_write( // https://github.com/denoland/deno/issues/3565 poll_fn(|cx| { let resource_table = &mut state.borrow_mut().resource_table; - let resource = resource_table - .get_mut::(rid as u32) + let resource_holder = resource_table + .get_mut::(rid as u32) .ok_or_else(OpError::bad_resource_id)?; - resource.poll_flush(cx) + resource_holder.resource.poll_flush(cx) }) .await?; diff --git a/cli/ops/net.rs b/cli/ops/net.rs index 50d6b371362af6..8b93f1366e1c7a 100644 --- a/cli/ops/net.rs +++ b/cli/ops/net.rs @@ -1,6 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; -use super::io::StreamResource; +use super::io::{StreamResource, StreamResourceHolder}; use crate::op_error::OpError; use crate::resolve_addr::resolve_addr; use crate::state::State; @@ -78,9 +78,12 @@ fn op_accept( let local_addr = tcp_stream.local_addr()?; let remote_addr = tcp_stream.peer_addr()?; let mut state = state_.borrow_mut(); - let rid = state - .resource_table - .add("tcpStream", Box::new(StreamResource::TcpStream(tcp_stream))); + let rid = state.resource_table.add( + "tcpStream", + Box::new(StreamResourceHolder::new(StreamResource::TcpStream( + tcp_stream, + ))), + ); Ok(json!({ "rid": rid, "localAddr": { @@ -203,9 +206,12 @@ fn op_connect( let local_addr = tcp_stream.local_addr()?; let remote_addr = tcp_stream.peer_addr()?; let mut state = state_.borrow_mut(); - let rid = state - .resource_table - .add("tcpStream", Box::new(StreamResource::TcpStream(tcp_stream))); + let rid = state.resource_table.add( + "tcpStream", + Box::new(StreamResourceHolder::new(StreamResource::TcpStream( + tcp_stream, + ))), + ); Ok(json!({ "rid": rid, "localAddr": { From 48276909bb0d3c8e09a1ad971b2ebc3d5f2bf531 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Mon, 9 Mar 2020 11:57:12 +0000 Subject: [PATCH 02/11] wip: wrap the remainder resources --- cli/ops/fetch.rs | 6 ++-- cli/ops/fs.rs | 13 +++++---- cli/ops/io.rs | 20 +++++++++---- cli/ops/net.rs | 6 ++-- cli/ops/process.rs | 20 ++++++++----- cli/ops/tls.rs | 10 +++++-- cli/ops/tty.rs | 73 ++++++++++++++++++++++++---------------------- 7 files changed, 87 insertions(+), 61 deletions(-) diff --git a/cli/ops/fetch.rs b/cli/ops/fetch.rs index 9f36ad5fd1c2e7..d222787a623e5b 100644 --- a/cli/ops/fetch.rs +++ b/cli/ops/fetch.rs @@ -1,6 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; -use super::io::StreamResource; +use super::io::{StreamResource, StreamResourceHolder}; use crate::http_util::{create_http_client, HttpBody}; use crate::op_error::OpError; use crate::state::State; @@ -80,7 +80,9 @@ pub fn op_fetch( let mut state = state_.borrow_mut(); let rid = state.resource_table.add( "httpBody", - Box::new(StreamResource::HttpBody(Box::new(body))), + Box::new(StreamResourceHolder::new(StreamResource::HttpBody( + Box::new(body), + ))), ); let json_res = json!({ diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs index ad1283c122230b..0c0ad67b01ca45 100644 --- a/cli/ops/fs.rs +++ b/cli/ops/fs.rs @@ -1,7 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. // Some deserializer fields are only used on Unix and Windows build fails without it use super::dispatch_json::{blocking_json, Deserialize, JsonOp, Value}; -use super::io::{FileMetadata, StreamResource}; +use super::io::{FileMetadata, StreamResource, StreamResourceHolder}; use crate::fs as deno_fs; use crate::op_error::OpError; use crate::ops::dispatch_json::JsonResult; @@ -152,7 +152,10 @@ fn op_open( let mut state = state_.borrow_mut(); let rid = state.resource_table.add( "fsFile", - Box::new(StreamResource::FsFile(fs_file, FileMetadata::default())), + Box::new(StreamResourceHolder::new(StreamResource::FsFile( + fs_file, + FileMetadata::default(), + ))), ); Ok(json!(rid)) }; @@ -197,12 +200,12 @@ fn op_seek( }; let state = state.borrow(); - let resource = state + let resource_holder = state .resource_table - .get::(rid) + .get::(rid) .ok_or_else(OpError::bad_resource_id)?; - let tokio_file = match resource { + let tokio_file = match resource_holder.resource { StreamResource::FsFile(ref file, _) => file, _ => return Err(OpError::bad_resource_id()), }; diff --git a/cli/ops/io.rs b/cli/ops/io.rs index 215f3617944f8b..e9ad77418b0095 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -56,15 +56,23 @@ pub fn init(i: &mut Isolate, s: &State) { ); } -pub fn get_stdio() -> (StreamResource, StreamResource, StreamResource) { - let stdin = StreamResource::Stdin(tokio::io::stdin(), TTYMetadata::default()); - let stdout = StreamResource::Stdout({ +pub fn get_stdio() -> ( + StreamResourceHolder, + StreamResourceHolder, + StreamResourceHolder, +) { + let stdin = StreamResourceHolder::new(StreamResource::Stdin( + tokio::io::stdin(), + TTYMetadata::default(), + )); + let stdout = StreamResourceHolder::new(StreamResource::Stdout({ let stdout = STDOUT_HANDLE .try_clone() .expect("Unable to clone stdout handle"); tokio::fs::File::from_std(stdout) - }); - let stderr = StreamResource::Stderr(tokio::io::stderr()); + })); + let stderr = + StreamResourceHolder::new(StreamResource::Stderr(tokio::io::stderr())); (stdin, stdout, stderr) } @@ -88,7 +96,7 @@ pub struct FileMetadata { } pub struct StreamResourceHolder { - resource: StreamResource, + pub resource: StreamResource, waker: Option, } diff --git a/cli/ops/net.rs b/cli/ops/net.rs index 8b93f1366e1c7a..b67dc921157def 100644 --- a/cli/ops/net.rs +++ b/cli/ops/net.rs @@ -253,11 +253,11 @@ fn op_shutdown( }; let mut state = state.borrow_mut(); - let resource = state + let resource_holder = state .resource_table - .get_mut::(rid) + .get_mut::(rid) .ok_or_else(OpError::bad_resource_id)?; - match resource { + match resource_holder.resource { StreamResource::TcpStream(ref mut stream) => { TcpStream::shutdown(stream, shutdown_mode).map_err(OpError::from)?; } diff --git a/cli/ops/process.rs b/cli/ops/process.rs index 743ffa22b4e9f1..55080fc2d23291 100644 --- a/cli/ops/process.rs +++ b/cli/ops/process.rs @@ -1,6 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; -use super::io::StreamResource; +use super::io::{StreamResource, StreamResourceHolder}; use crate::op_error::OpError; use crate::signal::kill; use crate::state::State; @@ -24,11 +24,11 @@ pub fn init(i: &mut Isolate, s: &State) { fn clone_file(rid: u32, state: &State) -> Result { let mut state = state.borrow_mut(); - let repr = state + let repr_holder = state .resource_table - .get_mut::(rid) + .get_mut::(rid) .ok_or_else(OpError::bad_resource_id)?; - let file = match repr { + let file = match repr_holder.resource { StreamResource::FsFile(ref mut file, _) => file, _ => return Err(OpError::bad_resource_id()), }; @@ -127,7 +127,9 @@ fn op_run( Some(child_stdin) => { let rid = table.add( "childStdin", - Box::new(StreamResource::ChildStdin(child_stdin)), + Box::new(StreamResourceHolder::new(StreamResource::ChildStdin( + child_stdin, + ))), ); Some(rid) } @@ -138,7 +140,9 @@ fn op_run( Some(child_stdout) => { let rid = table.add( "childStdout", - Box::new(StreamResource::ChildStdout(child_stdout)), + Box::new(StreamResourceHolder::new(StreamResource::ChildStdout( + child_stdout, + ))), ); Some(rid) } @@ -149,7 +153,9 @@ fn op_run( Some(child_stderr) => { let rid = table.add( "childStderr", - Box::new(StreamResource::ChildStderr(child_stderr)), + Box::new(StreamResourceHolder::new(StreamResource::ChildStderr( + child_stderr, + ))), ); Some(rid) } diff --git a/cli/ops/tls.rs b/cli/ops/tls.rs index da34a1a131e698..605554e9c5913c 100644 --- a/cli/ops/tls.rs +++ b/cli/ops/tls.rs @@ -1,6 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; -use super::io::StreamResource; +use super::io::{StreamResource, StreamResourceHolder}; use crate::op_error::OpError; use crate::resolve_addr::resolve_addr; use crate::state::State; @@ -85,7 +85,9 @@ pub fn op_connect_tls( let mut state = state_.borrow_mut(); let rid = state.resource_table.add( "clientTlsStream", - Box::new(StreamResource::ClientTlsStream(Box::new(tls_stream))), + Box::new(StreamResourceHolder::new(StreamResource::ClientTlsStream( + Box::new(tls_stream), + ))), ); Ok(json!({ "rid": rid, @@ -318,7 +320,9 @@ fn op_accept_tls( let mut state = state.borrow_mut(); state.resource_table.add( "serverTlsStream", - Box::new(StreamResource::ServerTlsStream(Box::new(tls_stream))), + Box::new(StreamResourceHolder::new(StreamResource::ServerTlsStream( + Box::new(tls_stream), + ))), ) }; Ok(json!({ diff --git a/cli/ops/tty.rs b/cli/ops/tty.rs index 69ac66688cfa07..78fd91d8360884 100644 --- a/cli/ops/tty.rs +++ b/cli/ops/tty.rs @@ -1,5 +1,5 @@ use super::dispatch_json::JsonOp; -use super::io::StreamResource; +use super::io::{StreamResource, StreamResourceHolder}; use crate::op_error::OpError; use crate::ops::json_op; use crate::state::State; @@ -66,13 +66,13 @@ pub fn op_set_raw( use winapi::um::{consoleapi, handleapi}; let state = state_.borrow_mut(); - let resource = state.resource_table.get::(rid); - if resource.is_none() { + let resource_holder = state.resource_table.get::(rid); + if resource_holder.is_none() { return Err(OpError::bad_resource_id()); } // For now, only stdin. - let handle = match resource.unwrap() { + let handle = match resource_holder.unwrap().resource { StreamResource::Stdin(_, _) => std::io::stdin().as_raw_handle(), StreamResource::FsFile(f, _) => { let tokio_file = futures::executor::block_on(f.try_clone())?; @@ -111,25 +111,27 @@ pub fn op_set_raw( use std::os::unix::io::AsRawFd; let mut state = state_.borrow_mut(); - let resource = state.resource_table.get_mut::(rid); - if resource.is_none() { + let resource_holder = + state.resource_table.get_mut::(rid); + if resource_holder.is_none() { return Err(OpError::bad_resource_id()); } if is_raw { - let (raw_fd, maybe_tty_mode) = match resource.unwrap() { - StreamResource::Stdin(_, ref mut metadata) => { - (std::io::stdin().as_raw_fd(), &mut metadata.mode) - } - StreamResource::FsFile(f, ref mut metadata) => { - let tokio_file = futures::executor::block_on(f.try_clone())?; - let std_file = futures::executor::block_on(tokio_file.into_std()); - (std_file.as_raw_fd(), &mut metadata.tty.mode) - } - _ => { - return Err(OpError::other("Not supported".to_owned())); - } - }; + let (raw_fd, maybe_tty_mode) = + match &mut resource_holder.unwrap().resource { + StreamResource::Stdin(_, ref mut metadata) => { + (std::io::stdin().as_raw_fd(), &mut metadata.mode) + } + StreamResource::FsFile(f, ref mut metadata) => { + let tokio_file = futures::executor::block_on(f.try_clone())?; + let std_file = futures::executor::block_on(tokio_file.into_std()); + (std_file.as_raw_fd(), &mut metadata.tty.mode) + } + _ => { + return Err(OpError::other("Not supported".to_owned())); + } + }; if maybe_tty_mode.is_some() { // Already raw. Skip. @@ -159,19 +161,20 @@ pub fn op_set_raw( Ok(JsonOp::Sync(json!({}))) } else { // Try restore saved mode. - let (raw_fd, maybe_tty_mode) = match resource.unwrap() { - StreamResource::Stdin(_, ref mut metadata) => { - (std::io::stdin().as_raw_fd(), &mut metadata.mode) - } - StreamResource::FsFile(f, ref mut metadata) => { - let tokio_file = futures::executor::block_on(f.try_clone())?; - let std_file = futures::executor::block_on(tokio_file.into_std()); - (std_file.as_raw_fd(), &mut metadata.tty.mode) - } - _ => { - return Err(OpError::other("Not supported".to_owned())); - } - }; + let (raw_fd, maybe_tty_mode) = + match &mut resource_holder.unwrap().resource { + StreamResource::Stdin(_, ref mut metadata) => { + (std::io::stdin().as_raw_fd(), &mut metadata.mode) + } + StreamResource::FsFile(f, ref mut metadata) => { + let tokio_file = futures::executor::block_on(f.try_clone())?; + let std_file = futures::executor::block_on(tokio_file.into_std()); + (std_file.as_raw_fd(), &mut metadata.tty.mode) + } + _ => { + return Err(OpError::other("Not supported".to_owned())); + } + }; if let Some(mode) = maybe_tty_mode.take() { termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &mode)?; @@ -200,12 +203,12 @@ pub fn op_isatty( return Err(OpError::bad_resource_id()); } - let resource = state.resource_table.get::(rid); - if resource.is_none() { + let resource_holder = state.resource_table.get::(rid); + if resource_holder.is_none() { return Ok(JsonOp::Sync(json!(false))); } - match resource.unwrap() { + match &resource_holder.unwrap().resource { StreamResource::Stdin(_, _) => { Ok(JsonOp::Sync(json!(atty::is(atty::Stream::Stdin)))) } From 448633c5b56de8e9a4bdda46068525798dddb539 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Tue, 10 Mar 2020 17:32:20 +0000 Subject: [PATCH 03/11] wip: implement multiple task waker --- cli/ops/io.rs | 53 ++++++++++++++++++++++++--------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/cli/ops/io.rs b/cli/ops/io.rs index e9ad77418b0095..d4b418e72b2dcf 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -7,7 +7,9 @@ use deno_core::*; use futures::future::poll_fn; use futures::future::FutureExt; use futures::ready; +use std::collections::HashMap; use std::pin::Pin; +use std::sync::atomic::{AtomicIsize, Ordering}; use std::task::Context; use std::task::Poll; use tokio::io::{AsyncRead, AsyncWrite}; @@ -97,56 +99,46 @@ pub struct FileMetadata { pub struct StreamResourceHolder { pub resource: StreamResource, - waker: Option, + waker: HashMap, + waker_counter: AtomicIsize, } impl StreamResourceHolder { pub fn new(resource: StreamResource) -> StreamResourceHolder { StreamResourceHolder { resource, - waker: None, + // Atleast one task is expecter for the resource + waker: HashMap::with_capacity(1), + // Tracks wakers Ids + waker_counter: AtomicIsize::new(0), } } } impl Drop for StreamResourceHolder { fn drop(&mut self) { - self.wake_task(); + self.wake_tasks(); } } impl StreamResourceHolder { - /// Track the current task so future awaiting for connection - /// can be notified when listener is closed. - /// - /// Throws an error if another task is already tracked. - pub fn track_task(&mut self, cx: &Context) -> Result<(), OpError> { - // Currently, we only allow tracking a single accept task for a listener. - // This might be changed in the future with multiple workers. - // Caveat: TcpListener by itself also only tracks an accept task at a time. - // See https://github.com/tokio-rs/tokio/issues/846#issuecomment-454208883 - if self.waker.is_some() { - return Err(OpError::other("Another accept task is ongoing".to_string())); - } + pub fn track_task(&mut self, cx: &Context) -> Result { let waker = futures::task::AtomicWaker::new(); waker.register(cx.waker()); - self.waker.replace(waker); - Ok(()) + // Its OK if it overflows + let task_waker_id = self.waker_counter.fetch_add(1, Ordering::Relaxed); + self.waker.insert(task_waker_id, waker); + Ok(task_waker_id) } - /// Notifies a task when listener is closed so accept future can resolve. - pub fn wake_task(&mut self) { - if let Some(waker) = self.waker.as_ref() { + pub fn wake_tasks(&mut self) { + for waker in self.waker.values() { waker.wake(); } } - /// Stop tracking a task. - /// Happens when the task is done and thus no further tracking is needed. - pub fn untrack_task(&mut self) { - if self.waker.is_some() { - self.waker.take(); - } + pub fn untrack_task(&mut self, task_waker_id: isize) { + self.waker.remove(&task_waker_id); } } @@ -210,22 +202,27 @@ pub fn op_read( let state = state.clone(); let mut buf = zero_copy.unwrap(); + // let mut task_tracker_id: Option = None; poll_fn(move |cx| { let resource_table = &mut state.borrow_mut().resource_table; let resource_holder = resource_table .get_mut::(rid as u32) .ok_or_else(OpError::bad_resource_id)?; + + let mut task_tracker_id: Option = None; let nread = match resource_holder .resource .poll_read(cx, &mut buf.as_mut()[..]) { Poll::Ready(t) => { - resource_holder.untrack_task(); + if let Some(id) = task_tracker_id { + resource_holder.untrack_task(id); + } t } Poll::Pending => { - resource_holder.track_task(cx)?; + task_tracker_id.replace(resource_holder.track_task(cx)?); return Poll::Pending; } }?; From a2dcb1e9ed3ea68a0618f27a76643f30c2f7c9b9 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Tue, 10 Mar 2020 18:05:27 +0000 Subject: [PATCH 04/11] fix: windows fix --- cli/ops/tty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ops/tty.rs b/cli/ops/tty.rs index 78fd91d8360884..c44ab946f722b2 100644 --- a/cli/ops/tty.rs +++ b/cli/ops/tty.rs @@ -72,7 +72,7 @@ pub fn op_set_raw( } // For now, only stdin. - let handle = match resource_holder.unwrap().resource { + let handle = match &resource_holder.unwrap().resource { StreamResource::Stdin(_, _) => std::io::stdin().as_raw_handle(), StreamResource::FsFile(f, _) => { let tokio_file = futures::executor::block_on(f.try_clone())?; From bfe8f01f1e32e98ef404fe5b475a7301fa73b0b5 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 00:02:28 +0000 Subject: [PATCH 05/11] fix: error mapping --- cli/ops/io.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ops/io.rs b/cli/ops/io.rs index d4b418e72b2dcf..7291fa86e58882 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -213,7 +213,7 @@ pub fn op_read( let mut task_tracker_id: Option = None; let nread = match resource_holder .resource - .poll_read(cx, &mut buf.as_mut()[..]) + .poll_read(cx, &mut buf.as_mut()[..]).map_err(OpError::from) { Poll::Ready(t) => { if let Some(id) = task_tracker_id { From c14ddd1a8589a122cb420d60396fdcc189693444 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 02:05:49 +0000 Subject: [PATCH 06/11] wip: add test & isize -> usize cleanup --- cli/ops/io.rs | 18 +++---- cli/tests/integration_tests.rs | 83 ++++++++++++++++++++++++------ cli/tests/listener_hanging_test.ts | 45 ++++++++++++++++ 3 files changed, 122 insertions(+), 24 deletions(-) create mode 100644 cli/tests/listener_hanging_test.ts diff --git a/cli/ops/io.rs b/cli/ops/io.rs index 7291fa86e58882..b7f67cea426df2 100644 --- a/cli/ops/io.rs +++ b/cli/ops/io.rs @@ -9,7 +9,7 @@ use futures::future::FutureExt; use futures::ready; use std::collections::HashMap; use std::pin::Pin; -use std::sync::atomic::{AtomicIsize, Ordering}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::task::Context; use std::task::Poll; use tokio::io::{AsyncRead, AsyncWrite}; @@ -99,8 +99,8 @@ pub struct FileMetadata { pub struct StreamResourceHolder { pub resource: StreamResource, - waker: HashMap, - waker_counter: AtomicIsize, + waker: HashMap, + waker_counter: AtomicUsize, } impl StreamResourceHolder { @@ -110,7 +110,7 @@ impl StreamResourceHolder { // Atleast one task is expecter for the resource waker: HashMap::with_capacity(1), // Tracks wakers Ids - waker_counter: AtomicIsize::new(0), + waker_counter: AtomicUsize::new(0), } } } @@ -122,7 +122,7 @@ impl Drop for StreamResourceHolder { } impl StreamResourceHolder { - pub fn track_task(&mut self, cx: &Context) -> Result { + pub fn track_task(&mut self, cx: &Context) -> Result { let waker = futures::task::AtomicWaker::new(); waker.register(cx.waker()); // Its OK if it overflows @@ -137,7 +137,7 @@ impl StreamResourceHolder { } } - pub fn untrack_task(&mut self, task_waker_id: isize) { + pub fn untrack_task(&mut self, task_waker_id: usize) { self.waker.remove(&task_waker_id); } } @@ -202,7 +202,6 @@ pub fn op_read( let state = state.clone(); let mut buf = zero_copy.unwrap(); - // let mut task_tracker_id: Option = None; poll_fn(move |cx| { let resource_table = &mut state.borrow_mut().resource_table; @@ -210,10 +209,11 @@ pub fn op_read( .get_mut::(rid as u32) .ok_or_else(OpError::bad_resource_id)?; - let mut task_tracker_id: Option = None; + let mut task_tracker_id: Option = None; let nread = match resource_holder .resource - .poll_read(cx, &mut buf.as_mut()[..]).map_err(OpError::from) + .poll_read(cx, &mut buf.as_mut()[..]) + .map_err(OpError::from) { Poll::Ready(t) => { if let Some(id) = task_tracker_id { diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 1e82742942fdf4..e48d099af1d269 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -477,6 +477,7 @@ fn repl_test_console_log() { Some(vec!["console.log('hello')", "'world'"]), None, false, + None, ); assert_eq!(out, "hello\nundefined\nworld\n"); assert!(err.is_empty()); @@ -490,6 +491,7 @@ fn repl_test_eof() { Some(vec!["1 + 2"]), None, false, + None, ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -503,6 +505,7 @@ fn repl_test_exit_command() { Some(vec!["exit", "'ignored'"]), None, false, + None, ); assert!(out.is_empty()); assert!(err.is_empty()); @@ -510,8 +513,14 @@ fn repl_test_exit_command() { #[test] fn repl_test_help_command() { - let (out, err) = - util::run_and_collect_output(true, "repl", Some(vec!["help"]), None, false); + let (out, err) = util::run_and_collect_output( + true, + "repl", + Some(vec!["help"]), + None, + false, + None, + ); assert_eq!( out, vec![ @@ -534,6 +543,7 @@ fn repl_test_function() { Some(vec!["Deno.writeFileSync"]), None, false, + None, ); assert_eq!(out, "[Function: writeFileSync]\n"); assert!(err.is_empty()); @@ -547,6 +557,7 @@ fn repl_test_multiline() { Some(vec!["(\n1 + 2\n)"]), None, false, + None, ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -560,6 +571,7 @@ fn repl_test_eval_unterminated() { Some(vec!["eval('{')"]), None, false, + None, ); assert!(out.is_empty()); assert!(err.contains("Unexpected end of input")); @@ -573,6 +585,7 @@ fn repl_test_reference_error() { Some(vec!["not_a_variable"]), None, false, + None, ); assert!(out.is_empty()); assert!(err.contains("not_a_variable is not defined")); @@ -586,6 +599,7 @@ fn repl_test_syntax_error() { Some(vec!["syntax error"]), None, false, + None, ); assert!(out.is_empty()); assert!(err.contains("Unexpected identifier")); @@ -599,6 +613,7 @@ fn repl_test_type_error() { Some(vec!["console()"]), None, false, + None, ); assert!(out.is_empty()); assert!(err.contains("console is not a function")); @@ -612,6 +627,7 @@ fn repl_test_variable() { Some(vec!["var a = 123;", "a"]), None, false, + None, ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -625,6 +641,7 @@ fn repl_test_lexical_scoped_variable() { Some(vec!["let a = 123;", "a"]), None, false, + None, ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -642,6 +659,7 @@ fn repl_test_missing_deno_dir() { Some(vec!["1"]), Some(vec![("DENO_DIR".to_owned(), DENO_DIR.to_owned())]), false, + None, ); assert!(read_dir(&test_deno_dir).is_ok()); remove_dir_all(&test_deno_dir).unwrap(); @@ -657,6 +675,7 @@ fn repl_test_save_last_eval() { Some(vec!["1", "_"]), None, false, + None, ); assert_eq!(out, "1\n1\n"); assert!(err.is_empty()); @@ -670,6 +689,7 @@ fn repl_test_save_last_thrown() { Some(vec!["throw 1", "_error"]), None, false, + None, ); assert_eq!(out, "1\n"); assert_eq!(err, "Thrown: 1\n"); @@ -683,6 +703,7 @@ fn repl_test_assign_underscore() { Some(vec!["_ = 1", "2", "_"]), None, false, + None, ); assert_eq!( out, @@ -699,6 +720,7 @@ fn repl_test_assign_underscore_error() { Some(vec!["_error = 1", "throw 2", "_error"]), None, false, + None, ); assert_eq!( out, @@ -1580,6 +1602,7 @@ fn test_permissions_with_allow() { None, None, false, + None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1594,6 +1617,7 @@ fn test_permissions_without_allow() { None, None, false, + None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1614,6 +1638,7 @@ fn test_permissions_rw_inside_project_dir() { None, None, false, + None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1643,6 +1668,7 @@ fn test_permissions_rw_outside_test_dir() { None, None, false, + None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1668,6 +1694,7 @@ fn test_permissions_rw_inside_test_dir() { None, None, false, + None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1704,6 +1731,7 @@ fn test_permissions_rw_outside_test_and_js_dir() { None, None, false, + None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1733,6 +1761,7 @@ fn test_permissions_rw_inside_test_and_js_dir() { None, None, false, + None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1751,6 +1780,7 @@ fn test_permissions_rw_relative() { None, None, false, + None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1768,7 +1798,7 @@ fn test_permissions_rw_no_prefix() { ), None, None, - false, + false,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1780,7 +1810,7 @@ fn test_permissions_net_fetch_allow_localhost_4545() { true, "run --allow-net=localhost:4545 complex_permissions_test.ts netFetch http://localhost:4545/", None, - None,true, + None,true,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1792,7 +1822,7 @@ fn test_permissions_net_fetch_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netFetch http://localhost:4545/", None, None, - true, + true,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1804,7 +1834,7 @@ fn test_permissions_net_fetch_localhost_4545_fail() { "run --allow-net=localhost:4545 complex_permissions_test.ts netFetch http://localhost:4546/", None, None, - true, + true,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1816,7 +1846,7 @@ fn test_permissions_net_fetch_localhost() { "run --allow-net=localhost complex_permissions_test.ts netFetch http://localhost:4545/ http://localhost:4546/ http://localhost:4547/", None, None, - true, + true,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1828,7 +1858,7 @@ fn test_permissions_net_connect_allow_localhost_ip_4555() { "run --allow-net=127.0.0.1:4545 complex_permissions_test.ts netConnect 127.0.0.1:4545", None, None, - true, + true,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1840,7 +1870,7 @@ fn test_permissions_net_connect_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netConnect 127.0.0.1:4546", None, None, - true, + true,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1852,7 +1882,7 @@ fn test_permissions_net_connect_allow_localhost_ip_4545_fail() { "run --allow-net=127.0.0.1:4545 complex_permissions_test.ts netConnect 127.0.0.1:4546", None, None, - true, + true,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1864,7 +1894,7 @@ fn test_permissions_net_connect_allow_localhost_ip() { "run --allow-net=127.0.0.1 complex_permissions_test.ts netConnect 127.0.0.1:4545 127.0.0.1:4546 127.0.0.1:4547", None, None, - true, + true,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1876,7 +1906,7 @@ fn test_permissions_net_listen_allow_localhost_4555() { "run --allow-net=localhost:4558 complex_permissions_test.ts netListen localhost:4558", None, None, - false, + false,None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1888,7 +1918,7 @@ fn test_permissions_net_listen_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netListen localhost:4545", None, None, - false, + false,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1900,7 +1930,7 @@ fn test_permissions_net_listen_allow_localhost_4555_fail() { "run --allow-net=localhost:4555 complex_permissions_test.ts netListen localhost:4556", None, None, - false, + false,None ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1913,11 +1943,24 @@ fn test_permissions_net_listen_allow_localhost() { "run --allow-net=localhost complex_permissions_test.ts netListen localhost:4600", None, None, - false, + false, + None ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } +#[test] +fn test_read_after_close_hang() { + util::run_and_collect_output( + true, + "run --allow-net listener_hanging_test.ts", + None, + None, + false, + Some(2500), + ); +} + mod util { use deno::colors::strip_ansi_codes; pub use deno::test_util::*; @@ -1927,6 +1970,8 @@ mod util { use std::process::Command; use std::process::Output; use std::process::Stdio; + use std::{thread, time}; + use tempfile::TempDir; pub const PERMISSION_VARIANTS: [&str; 5] = @@ -1943,6 +1988,7 @@ mod util { input: Option>, envs: Option>, need_http_server: bool, + timeout: Option, ) -> (String, String) { let root = root_path(); let tests_dir = root.join("cli").join("tests"); @@ -1970,6 +2016,13 @@ mod util { .write_all(lines.join("\n").as_bytes()) .expect("failed to write to stdin"); } + if let Some(timeout) = timeout { + let ten_millis = time::Duration::from_millis(timeout); + thread::sleep(ten_millis); + if deno.try_wait().unwrap().is_none() { + panic!("Timed out!"); + } + } let Output { stdout, stderr, diff --git a/cli/tests/listener_hanging_test.ts b/cli/tests/listener_hanging_test.ts new file mode 100644 index 00000000000000..9ce9ef91f51885 --- /dev/null +++ b/cli/tests/listener_hanging_test.ts @@ -0,0 +1,45 @@ +async function server(): Promise { + const l = Deno.listen({ port: 4444 }); + const buf = new Uint8Array(4); + const conn = await l.accept(); + const process = async function*(): any { + while (true) { + // Read request with timeout + const nr = await Promise.race([ + conn.read(buf), + new Promise(resolve => { + setTimeout(resolve, 100); + }) + ]); + if (!nr) { + conn.close(); + return; + } else { + await conn.write(new Uint8Array([0, 1, 2, 3])); + } + } + }; + for await (const _ of process()); + l.close(); +} +server(); + +const conn = await Deno.connect({ port: 4444 }); +async function reqRes(): Promise { + await conn.write(new Uint8Array([0, 1, 2, 3])); + const buf = new Uint8Array(4); + await conn.read(buf); + return; +} +// First request-response: +await reqRes(); +// Second request-response: expect error as conn is closed by server +setTimeout(async () => { + try { + await reqRes(); + } catch (e) { + console.error(e); + } finally { + conn.close(); + } +}, 200); From 262af6343153a93fa19b28e693a2221fe3c234c5 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 09:17:09 +0000 Subject: [PATCH 07/11] wip: cleaup typescript test file --- cli/tests/listener_hanging_test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cli/tests/listener_hanging_test.ts b/cli/tests/listener_hanging_test.ts index 9ce9ef91f51885..48f59838924f70 100644 --- a/cli/tests/listener_hanging_test.ts +++ b/cli/tests/listener_hanging_test.ts @@ -2,10 +2,11 @@ async function server(): Promise { const l = Deno.listen({ port: 4444 }); const buf = new Uint8Array(4); const conn = await l.accept(); - const process = async function*(): any { + const process = async function(): Promise { while (true) { - // Read request with timeout const nr = await Promise.race([ + // Testing multiple read tasks! + conn.read(buf), conn.read(buf), new Promise(resolve => { setTimeout(resolve, 100); @@ -19,7 +20,7 @@ async function server(): Promise { } } }; - for await (const _ of process()); + await process(); l.close(); } server(); @@ -29,7 +30,6 @@ async function reqRes(): Promise { await conn.write(new Uint8Array([0, 1, 2, 3])); const buf = new Uint8Array(4); await conn.read(buf); - return; } // First request-response: await reqRes(); From d86d2937628f90d3f8b390f1425f9d9ed061c719 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 09:35:40 +0000 Subject: [PATCH 08/11] increase timeout --- cli/tests/integration_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index e48d099af1d269..7596118521b32a 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1957,7 +1957,7 @@ fn test_read_after_close_hang() { None, None, false, - Some(2500), + Some(10000), ); } From 5ce2333fc407a5fa6cc1581bd72dc7013f994b17 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 11:16:04 +0000 Subject: [PATCH 09/11] wip: revert test & implement new on net_test --- cli/js/tests/net_test.ts | 48 ++++++++++++++++ cli/tests/integration_tests.rs | 90 ++++++++++-------------------- cli/tests/listener_hanging_test.ts | 45 --------------- 3 files changed, 79 insertions(+), 104 deletions(-) delete mode 100644 cli/tests/listener_hanging_test.ts diff --git a/cli/js/tests/net_test.ts b/cli/js/tests/net_test.ts index 1a58c353110a73..5d08d55950f3ad 100644 --- a/cli/js/tests/net_test.ts +++ b/cli/js/tests/net_test.ts @@ -336,3 +336,51 @@ unitTest( conn.close(); } ); + +unitTest( + { + perms: { net: true } + }, + async function netHangsOnClose() { + let acceptedConn: Deno.Conn; + const resolvable = createResolvable(); + + async function iteratorReq(listener: Deno.Listener): Promise { + const p = new Uint8Array(10); + const conn = await listener.accept(); + acceptedConn = conn; + + try { + while (true) { + const nread = await conn.read(p); + console.log("read", nread, "bytes"); + if (nread === Deno.EOF) { + break; + } + const nwritten = await conn.write(new Uint8Array([1, 2, 3])); + console.log("written", nwritten, "bytes"); + } + } catch (err) { + assert(!!err); + assert(err instanceof Deno.errors.BadResource); + } + + resolvable.resolve(); + } + + const addr = { hostname: "127.0.0.1", port: 4500 }; + const listener = Deno.listen(addr); + iteratorReq(listener); + const conn = await Deno.connect(addr); + await conn.write(new Uint8Array([1, 2, 3, 4])); + const buf = new Uint8Array(10); + const nread = await conn.read(buf); + console.log("main read", nread); + console.log("accepted", acceptedConn!); + conn!.close(); + acceptedConn!.close(); + listener.close(); + console.log("heyt"); + await resolvable; + } +); diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 7596118521b32a..df3c1587ee8ab5 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -477,7 +477,7 @@ fn repl_test_console_log() { Some(vec!["console.log('hello')", "'world'"]), None, false, - None, + ); assert_eq!(out, "hello\nundefined\nworld\n"); assert!(err.is_empty()); @@ -491,7 +491,7 @@ fn repl_test_eof() { Some(vec!["1 + 2"]), None, false, - None, + ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -505,7 +505,7 @@ fn repl_test_exit_command() { Some(vec!["exit", "'ignored'"]), None, false, - None, + ); assert!(out.is_empty()); assert!(err.is_empty()); @@ -519,7 +519,7 @@ fn repl_test_help_command() { Some(vec!["help"]), None, false, - None, + ); assert_eq!( out, @@ -543,7 +543,7 @@ fn repl_test_function() { Some(vec!["Deno.writeFileSync"]), None, false, - None, + ); assert_eq!(out, "[Function: writeFileSync]\n"); assert!(err.is_empty()); @@ -557,7 +557,7 @@ fn repl_test_multiline() { Some(vec!["(\n1 + 2\n)"]), None, false, - None, + ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -571,7 +571,7 @@ fn repl_test_eval_unterminated() { Some(vec!["eval('{')"]), None, false, - None, + ); assert!(out.is_empty()); assert!(err.contains("Unexpected end of input")); @@ -585,7 +585,7 @@ fn repl_test_reference_error() { Some(vec!["not_a_variable"]), None, false, - None, + ); assert!(out.is_empty()); assert!(err.contains("not_a_variable is not defined")); @@ -599,7 +599,7 @@ fn repl_test_syntax_error() { Some(vec!["syntax error"]), None, false, - None, + ); assert!(out.is_empty()); assert!(err.contains("Unexpected identifier")); @@ -613,7 +613,7 @@ fn repl_test_type_error() { Some(vec!["console()"]), None, false, - None, + ); assert!(out.is_empty()); assert!(err.contains("console is not a function")); @@ -627,7 +627,7 @@ fn repl_test_variable() { Some(vec!["var a = 123;", "a"]), None, false, - None, + ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -641,7 +641,7 @@ fn repl_test_lexical_scoped_variable() { Some(vec!["let a = 123;", "a"]), None, false, - None, + ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -659,7 +659,7 @@ fn repl_test_missing_deno_dir() { Some(vec!["1"]), Some(vec![("DENO_DIR".to_owned(), DENO_DIR.to_owned())]), false, - None, + ); assert!(read_dir(&test_deno_dir).is_ok()); remove_dir_all(&test_deno_dir).unwrap(); @@ -675,7 +675,7 @@ fn repl_test_save_last_eval() { Some(vec!["1", "_"]), None, false, - None, + ); assert_eq!(out, "1\n1\n"); assert!(err.is_empty()); @@ -689,7 +689,7 @@ fn repl_test_save_last_thrown() { Some(vec!["throw 1", "_error"]), None, false, - None, + ); assert_eq!(out, "1\n"); assert_eq!(err, "Thrown: 1\n"); @@ -703,7 +703,7 @@ fn repl_test_assign_underscore() { Some(vec!["_ = 1", "2", "_"]), None, false, - None, + ); assert_eq!( out, @@ -720,7 +720,7 @@ fn repl_test_assign_underscore_error() { Some(vec!["_error = 1", "throw 2", "_error"]), None, false, - None, + ); assert_eq!( out, @@ -1602,7 +1602,6 @@ fn test_permissions_with_allow() { None, None, false, - None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1617,7 +1616,6 @@ fn test_permissions_without_allow() { None, None, false, - None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1638,7 +1636,6 @@ fn test_permissions_rw_inside_project_dir() { None, None, false, - None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1668,7 +1665,6 @@ fn test_permissions_rw_outside_test_dir() { None, None, false, - None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1694,7 +1690,6 @@ fn test_permissions_rw_inside_test_dir() { None, None, false, - None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1731,7 +1726,6 @@ fn test_permissions_rw_outside_test_and_js_dir() { None, None, false, - None, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1761,7 +1755,6 @@ fn test_permissions_rw_inside_test_and_js_dir() { None, None, false, - None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1780,7 +1773,6 @@ fn test_permissions_rw_relative() { None, None, false, - None, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1798,7 +1790,7 @@ fn test_permissions_rw_no_prefix() { ), None, None, - false,None + false, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1810,7 +1802,8 @@ fn test_permissions_net_fetch_allow_localhost_4545() { true, "run --allow-net=localhost:4545 complex_permissions_test.ts netFetch http://localhost:4545/", None, - None,true,None + None, + true, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1822,7 +1815,7 @@ fn test_permissions_net_fetch_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netFetch http://localhost:4545/", None, None, - true,None + true, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1834,7 +1827,7 @@ fn test_permissions_net_fetch_localhost_4545_fail() { "run --allow-net=localhost:4545 complex_permissions_test.ts netFetch http://localhost:4546/", None, None, - true,None + true, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1846,7 +1839,7 @@ fn test_permissions_net_fetch_localhost() { "run --allow-net=localhost complex_permissions_test.ts netFetch http://localhost:4545/ http://localhost:4546/ http://localhost:4547/", None, None, - true,None + true, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1858,7 +1851,7 @@ fn test_permissions_net_connect_allow_localhost_ip_4555() { "run --allow-net=127.0.0.1:4545 complex_permissions_test.ts netConnect 127.0.0.1:4545", None, None, - true,None + true, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1870,7 +1863,7 @@ fn test_permissions_net_connect_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netConnect 127.0.0.1:4546", None, None, - true,None + true, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1882,7 +1875,7 @@ fn test_permissions_net_connect_allow_localhost_ip_4545_fail() { "run --allow-net=127.0.0.1:4545 complex_permissions_test.ts netConnect 127.0.0.1:4546", None, None, - true,None + true, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1894,7 +1887,7 @@ fn test_permissions_net_connect_allow_localhost_ip() { "run --allow-net=127.0.0.1 complex_permissions_test.ts netConnect 127.0.0.1:4545 127.0.0.1:4546 127.0.0.1:4547", None, None, - true,None + true, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1906,7 +1899,7 @@ fn test_permissions_net_listen_allow_localhost_4555() { "run --allow-net=localhost:4558 complex_permissions_test.ts netListen localhost:4558", None, None, - false,None + false, ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1918,7 +1911,7 @@ fn test_permissions_net_listen_allow_deno_land() { "run --allow-net=deno.land complex_permissions_test.ts netListen localhost:4545", None, None, - false,None + false, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1930,7 +1923,7 @@ fn test_permissions_net_listen_allow_localhost_4555_fail() { "run --allow-net=localhost:4555 complex_permissions_test.ts netListen localhost:4556", None, None, - false,None + false, ); assert!(err.contains(util::PERMISSION_DENIED_PATTERN)); } @@ -1944,23 +1937,11 @@ fn test_permissions_net_listen_allow_localhost() { None, None, false, - None + ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } -#[test] -fn test_read_after_close_hang() { - util::run_and_collect_output( - true, - "run --allow-net listener_hanging_test.ts", - None, - None, - false, - Some(10000), - ); -} - mod util { use deno::colors::strip_ansi_codes; pub use deno::test_util::*; @@ -1970,7 +1951,6 @@ mod util { use std::process::Command; use std::process::Output; use std::process::Stdio; - use std::{thread, time}; use tempfile::TempDir; @@ -1988,7 +1968,6 @@ mod util { input: Option>, envs: Option>, need_http_server: bool, - timeout: Option, ) -> (String, String) { let root = root_path(); let tests_dir = root.join("cli").join("tests"); @@ -2016,13 +1995,6 @@ mod util { .write_all(lines.join("\n").as_bytes()) .expect("failed to write to stdin"); } - if let Some(timeout) = timeout { - let ten_millis = time::Duration::from_millis(timeout); - thread::sleep(ten_millis); - if deno.try_wait().unwrap().is_none() { - panic!("Timed out!"); - } - } let Output { stdout, stderr, diff --git a/cli/tests/listener_hanging_test.ts b/cli/tests/listener_hanging_test.ts deleted file mode 100644 index 48f59838924f70..00000000000000 --- a/cli/tests/listener_hanging_test.ts +++ /dev/null @@ -1,45 +0,0 @@ -async function server(): Promise { - const l = Deno.listen({ port: 4444 }); - const buf = new Uint8Array(4); - const conn = await l.accept(); - const process = async function(): Promise { - while (true) { - const nr = await Promise.race([ - // Testing multiple read tasks! - conn.read(buf), - conn.read(buf), - new Promise(resolve => { - setTimeout(resolve, 100); - }) - ]); - if (!nr) { - conn.close(); - return; - } else { - await conn.write(new Uint8Array([0, 1, 2, 3])); - } - } - }; - await process(); - l.close(); -} -server(); - -const conn = await Deno.connect({ port: 4444 }); -async function reqRes(): Promise { - await conn.write(new Uint8Array([0, 1, 2, 3])); - const buf = new Uint8Array(4); - await conn.read(buf); -} -// First request-response: -await reqRes(); -// Second request-response: expect error as conn is closed by server -setTimeout(async () => { - try { - await reqRes(); - } catch (e) { - console.error(e); - } finally { - conn.close(); - } -}, 200); From 89dca095a48fe5fae336a1abd0f254948c36d497 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 11:17:56 +0000 Subject: [PATCH 10/11] wip: format & lint --- cli/tests/integration_tests.rs | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index df3c1587ee8ab5..624c548148b76b 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -477,7 +477,6 @@ fn repl_test_console_log() { Some(vec!["console.log('hello')", "'world'"]), None, false, - ); assert_eq!(out, "hello\nundefined\nworld\n"); assert!(err.is_empty()); @@ -491,7 +490,6 @@ fn repl_test_eof() { Some(vec!["1 + 2"]), None, false, - ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -505,7 +503,6 @@ fn repl_test_exit_command() { Some(vec!["exit", "'ignored'"]), None, false, - ); assert!(out.is_empty()); assert!(err.is_empty()); @@ -513,14 +510,8 @@ fn repl_test_exit_command() { #[test] fn repl_test_help_command() { - let (out, err) = util::run_and_collect_output( - true, - "repl", - Some(vec!["help"]), - None, - false, - - ); + let (out, err) = + util::run_and_collect_output(true, "repl", Some(vec!["help"]), None, false); assert_eq!( out, vec![ @@ -543,7 +534,6 @@ fn repl_test_function() { Some(vec!["Deno.writeFileSync"]), None, false, - ); assert_eq!(out, "[Function: writeFileSync]\n"); assert!(err.is_empty()); @@ -557,7 +547,6 @@ fn repl_test_multiline() { Some(vec!["(\n1 + 2\n)"]), None, false, - ); assert_eq!(out, "3\n"); assert!(err.is_empty()); @@ -571,7 +560,6 @@ fn repl_test_eval_unterminated() { Some(vec!["eval('{')"]), None, false, - ); assert!(out.is_empty()); assert!(err.contains("Unexpected end of input")); @@ -585,7 +573,6 @@ fn repl_test_reference_error() { Some(vec!["not_a_variable"]), None, false, - ); assert!(out.is_empty()); assert!(err.contains("not_a_variable is not defined")); @@ -599,7 +586,6 @@ fn repl_test_syntax_error() { Some(vec!["syntax error"]), None, false, - ); assert!(out.is_empty()); assert!(err.contains("Unexpected identifier")); @@ -613,7 +599,6 @@ fn repl_test_type_error() { Some(vec!["console()"]), None, false, - ); assert!(out.is_empty()); assert!(err.contains("console is not a function")); @@ -627,7 +612,6 @@ fn repl_test_variable() { Some(vec!["var a = 123;", "a"]), None, false, - ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -641,7 +625,6 @@ fn repl_test_lexical_scoped_variable() { Some(vec!["let a = 123;", "a"]), None, false, - ); assert_eq!(out, "undefined\n123\n"); assert!(err.is_empty()); @@ -659,7 +642,6 @@ fn repl_test_missing_deno_dir() { Some(vec!["1"]), Some(vec![("DENO_DIR".to_owned(), DENO_DIR.to_owned())]), false, - ); assert!(read_dir(&test_deno_dir).is_ok()); remove_dir_all(&test_deno_dir).unwrap(); @@ -675,7 +657,6 @@ fn repl_test_save_last_eval() { Some(vec!["1", "_"]), None, false, - ); assert_eq!(out, "1\n1\n"); assert!(err.is_empty()); @@ -689,7 +670,6 @@ fn repl_test_save_last_thrown() { Some(vec!["throw 1", "_error"]), None, false, - ); assert_eq!(out, "1\n"); assert_eq!(err, "Thrown: 1\n"); @@ -703,7 +683,6 @@ fn repl_test_assign_underscore() { Some(vec!["_ = 1", "2", "_"]), None, false, - ); assert_eq!( out, @@ -720,7 +699,6 @@ fn repl_test_assign_underscore_error() { Some(vec!["_error = 1", "throw 2", "_error"]), None, false, - ); assert_eq!( out, @@ -1937,7 +1915,6 @@ fn test_permissions_net_listen_allow_localhost() { None, None, false, - ); assert!(!err.contains(util::PERMISSION_DENIED_PATTERN)); } From 0d1e67e5eed9e50e472c041b6b4688708efc83c5 Mon Sep 17 00:00:00 2001 From: jsouto18 Date: Wed, 11 Mar 2020 14:33:54 +0000 Subject: [PATCH 11/11] wip: cleanup test --- cli/js/tests/net_test.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/cli/js/tests/net_test.ts b/cli/js/tests/net_test.ts index 5d08d55950f3ad..fccd62f3899a27 100644 --- a/cli/js/tests/net_test.ts +++ b/cli/js/tests/net_test.ts @@ -353,12 +353,10 @@ unitTest( try { while (true) { const nread = await conn.read(p); - console.log("read", nread, "bytes"); if (nread === Deno.EOF) { break; } - const nwritten = await conn.write(new Uint8Array([1, 2, 3])); - console.log("written", nwritten, "bytes"); + await conn.write(new Uint8Array([1, 2, 3])); } } catch (err) { assert(!!err); @@ -374,13 +372,10 @@ unitTest( const conn = await Deno.connect(addr); await conn.write(new Uint8Array([1, 2, 3, 4])); const buf = new Uint8Array(10); - const nread = await conn.read(buf); - console.log("main read", nread); - console.log("accepted", acceptedConn!); + await conn.read(buf); conn!.close(); acceptedConn!.close(); listener.close(); - console.log("heyt"); await resolvable; } );