From 81f809f2a675ff4ff7f93231ca87a18cb5b4628e Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 16 Aug 2019 11:05:24 -0400 Subject: Revert "Remove dead code: legacy read/write ops" This is causing a segfault for unknown reasons - see #2787. This reverts commit 498f6ad431478f655b136782093e19e29248b24d. --- cli/dispatch_minimal.rs | 162 ++++++++++++++++++++++++++++++++++++++++++++ cli/main.rs | 1 + cli/msg.fbs | 22 ++++++ cli/ops/dispatch_minimal.rs | 110 ------------------------------ cli/ops/files.rs | 86 +++++++++++++++++++++++ cli/ops/io.rs | 43 ------------ cli/ops/mod.rs | 28 +++----- js/dispatch.ts | 6 +- js/dispatch_minimal.ts | 13 ---- js/files.ts | 56 ++++++++++----- 10 files changed, 322 insertions(+), 205 deletions(-) create mode 100644 cli/dispatch_minimal.rs delete mode 100644 cli/ops/dispatch_minimal.rs delete mode 100644 cli/ops/io.rs diff --git a/cli/dispatch_minimal.rs b/cli/dispatch_minimal.rs new file mode 100644 index 000000000..322094be2 --- /dev/null +++ b/cli/dispatch_minimal.rs @@ -0,0 +1,162 @@ +// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +// Do not add flatbuffer dependencies to this module. +//! Connects to js/dispatch_minimal.ts sendAsyncMinimal This acts as a faster +//! alternative to flatbuffers using a very simple list of int32s to lay out +//! messages. The first i32 is used to determine if a message a flatbuffer +//! message or a "minimal" message. +use crate::state::ThreadSafeState; +use deno::Buf; +use deno::CoreOp; +use deno::Op; +use deno::OpId; +use deno::PinnedBuf; +use futures::Future; + +const OP_READ: OpId = 1; +const OP_WRITE: OpId = 2; + +#[derive(Copy, Clone, Debug, PartialEq)] +// This corresponds to RecordMinimal on the TS side. +pub struct Record { + pub promise_id: i32, + pub arg: i32, + pub result: i32, +} + +impl Into for Record { + fn into(self) -> Buf { + let vec = vec![self.promise_id, self.arg, self.result]; + let buf32 = vec.into_boxed_slice(); + let ptr = Box::into_raw(buf32) as *mut [u8; 3 * 4]; + unsafe { Box::from_raw(ptr) } + } +} + +pub fn parse_min_record(bytes: &[u8]) -> Option { + if bytes.len() % std::mem::size_of::() != 0 { + return None; + } + let p = bytes.as_ptr(); + #[allow(clippy::cast_ptr_alignment)] + let p32 = p as *const i32; + let s = unsafe { std::slice::from_raw_parts(p32, bytes.len() / 4) }; + + if s.len() != 3 { + return None; + } + let ptr = s.as_ptr(); + let ints = unsafe { std::slice::from_raw_parts(ptr, 3) }; + Some(Record { + promise_id: ints[0], + arg: ints[1], + result: ints[2], + }) +} + +#[test] +fn test_parse_min_record() { + let buf = vec![1, 0, 0, 0, 3, 0, 0, 0, 4, 0, 0, 0]; + assert_eq!( + parse_min_record(&buf), + Some(Record { + promise_id: 1, + arg: 3, + result: 4, + }) + ); + + let buf = vec![]; + assert_eq!(parse_min_record(&buf), None); + + let buf = vec![5]; + assert_eq!(parse_min_record(&buf), None); +} + +pub fn dispatch_minimal( + state: &ThreadSafeState, + op_id: OpId, + mut record: Record, + zero_copy: Option, +) -> CoreOp { + let is_sync = record.promise_id == 0; + let min_op = match op_id { + OP_READ => ops::read(record.arg, zero_copy), + OP_WRITE => ops::write(record.arg, zero_copy), + _ => unimplemented!(), + }; + + let state = state.clone(); + + let fut = Box::new(min_op.then(move |result| -> Result { + match result { + Ok(r) => { + record.result = r; + } + Err(err) => { + // TODO(ry) The dispatch_minimal doesn't properly pipe errors back to + // the caller. + debug!("swallowed err {}", err); + record.result = -1; + } + } + let buf: Buf = record.into(); + state.metrics_op_completed(buf.len()); + Ok(buf) + })); + if is_sync { + Op::Sync(fut.wait().unwrap()) + } else { + Op::Async(fut) + } +} + +mod ops { + use crate::deno_error; + use crate::resources; + use crate::tokio_write; + use deno::ErrBox; + use deno::PinnedBuf; + use futures::Future; + + type MinimalOp = dyn Future + Send; + + pub fn read(rid: i32, zero_copy: Option) -> Box { + debug!("read rid={}", rid); + let zero_copy = match zero_copy { + None => { + return Box::new( + futures::future::err(deno_error::no_buffer_specified()), + ) + } + Some(buf) => buf, + }; + match resources::lookup(rid as u32) { + None => Box::new(futures::future::err(deno_error::bad_resource())), + Some(resource) => Box::new( + tokio::io::read(resource, zero_copy) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nread)| Ok(nread as i32)), + ), + } + } + + pub fn write(rid: i32, zero_copy: Option) -> Box { + debug!("write rid={}", rid); + let zero_copy = match zero_copy { + None => { + return Box::new( + futures::future::err(deno_error::no_buffer_specified()), + ) + } + Some(buf) => buf, + }; + match resources::lookup(rid as u32) { + None => Box::new(futures::future::err(deno_error::bad_resource())), + Some(resource) => Box::new( + tokio_write::write(resource, zero_copy) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nwritten)| Ok(nwritten as i32)), + ), + } + } +} diff --git a/cli/main.rs b/cli/main.rs index 2d3c70c97..01469e442 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -21,6 +21,7 @@ pub mod deno_dir; pub mod deno_error; pub mod diagnostics; mod disk_cache; +mod dispatch_minimal; mod file_fetcher; pub mod flags; pub mod fmt_errors; diff --git a/cli/msg.fbs b/cli/msg.fbs index def17fed3..8fb92fee9 100644 --- a/cli/msg.fbs +++ b/cli/msg.fbs @@ -48,8 +48,10 @@ union Any { PermissionRevoke, Permissions, PermissionsRes, + Read, ReadDir, ReadDirRes, + ReadRes, Readlink, ReadlinkRes, Remove, @@ -81,6 +83,8 @@ union Any { WorkerGetMessage, WorkerGetMessageRes, WorkerPostMessage, + Write, + WriteRes, } enum ErrorKind: byte { @@ -487,6 +491,24 @@ table OpenRes { rid: uint32; } +table Read { + rid: uint32; + // (ptr, len) is passed as second parameter to Deno.core.send(). +} + +table ReadRes { + nread: uint; + eof: bool; +} + +table Write { + rid: uint32; +} + +table WriteRes { + nbyte: uint; +} + table Close { rid: uint32; } diff --git a/cli/ops/dispatch_minimal.rs b/cli/ops/dispatch_minimal.rs deleted file mode 100644 index be2cafea4..000000000 --- a/cli/ops/dispatch_minimal.rs +++ /dev/null @@ -1,110 +0,0 @@ -// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. -// Do not add flatbuffer dependencies to this module. -//! Connects to js/dispatch_minimal.ts sendAsyncMinimal This acts as a faster -//! alternative to flatbuffers using a very simple list of int32s to lay out -//! messages. The first i32 is used to determine if a message a flatbuffer -//! message or a "minimal" message. -use crate::state::ThreadSafeState; -use deno::Buf; -use deno::CoreOp; -use deno::ErrBox; -use deno::Op; -use deno::PinnedBuf; -use futures::Future; - -#[derive(Copy, Clone, Debug, PartialEq)] -// This corresponds to RecordMinimal on the TS side. -pub struct Record { - pub promise_id: i32, - pub arg: i32, - pub result: i32, -} - -impl Into for Record { - fn into(self) -> Buf { - let vec = vec![self.promise_id, self.arg, self.result]; - let buf32 = vec.into_boxed_slice(); - let ptr = Box::into_raw(buf32) as *mut [u8; 3 * 4]; - unsafe { Box::from_raw(ptr) } - } -} - -pub fn parse_min_record(bytes: &[u8]) -> Option { - if bytes.len() % std::mem::size_of::() != 0 { - return None; - } - let p = bytes.as_ptr(); - #[allow(clippy::cast_ptr_alignment)] - let p32 = p as *const i32; - let s = unsafe { std::slice::from_raw_parts(p32, bytes.len() / 4) }; - - if s.len() != 3 { - return None; - } - let ptr = s.as_ptr(); - let ints = unsafe { std::slice::from_raw_parts(ptr, 3) }; - Some(Record { - promise_id: ints[0], - arg: ints[1], - result: ints[2], - }) -} - -#[test] -fn test_parse_min_record() { - let buf = vec![1, 0, 0, 0, 3, 0, 0, 0, 4, 0, 0, 0]; - assert_eq!( - parse_min_record(&buf), - Some(Record { - promise_id: 1, - arg: 3, - result: 4, - }) - ); - - let buf = vec![]; - assert_eq!(parse_min_record(&buf), None); - - let buf = vec![5]; - assert_eq!(parse_min_record(&buf), None); -} - -pub type MinimalOp = dyn Future + Send; -pub type Dispatcher = fn(i32, Option) -> Box; - -pub fn dispatch( - d: Dispatcher, - state: &ThreadSafeState, - control: &[u8], - zero_copy: Option, -) -> CoreOp { - let mut record = parse_min_record(control).unwrap(); - let is_sync = record.promise_id == 0; - - let state = state.clone(); - - let rid = record.arg; - let min_op = d(rid, zero_copy); - - let fut = Box::new(min_op.then(move |result| -> Result { - match result { - Ok(r) => { - record.result = r; - } - Err(err) => { - // TODO(ry) The dispatch_minimal doesn't properly pipe errors back to - // the caller. - debug!("swallowed err {}", err); - record.result = -1; - } - } - let buf: Buf = record.into(); - state.metrics_op_completed(buf.len()); - Ok(buf) - })); - if is_sync { - Op::Sync(fut.wait().unwrap()) - } else { - Op::Async(fut) - } -} diff --git a/cli/ops/files.rs b/cli/ops/files.rs index e311db425..ce3285623 100644 --- a/cli/ops/files.rs +++ b/cli/ops/files.rs @@ -8,6 +8,7 @@ use crate::ops::serialize_response; use crate::ops::CliOpResult; use crate::resources; use crate::state::ThreadSafeState; +use crate::tokio_write; use deno::*; use flatbuffers::FlatBufferBuilder; use futures::Future; @@ -118,6 +119,91 @@ pub fn op_close( } } +pub fn op_read( + _state: &ThreadSafeState, + base: &msg::Base<'_>, + data: Option, +) -> CliOpResult { + let cmd_id = base.cmd_id(); + let inner = base.inner_as_read().unwrap(); + let rid = inner.rid(); + + match resources::lookup(rid) { + None => Err(deno_error::bad_resource()), + Some(resource) => { + let op = tokio::io::read(resource, data.unwrap()) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nread)| { + let builder = &mut FlatBufferBuilder::new(); + let inner = msg::ReadRes::create( + builder, + &msg::ReadResArgs { + nread: nread as u32, + eof: nread == 0, + }, + ); + Ok(serialize_response( + cmd_id, + builder, + msg::BaseArgs { + inner: Some(inner.as_union_value()), + inner_type: msg::Any::ReadRes, + ..Default::default() + }, + )) + }); + if base.sync() { + let buf = op.wait()?; + Ok(Op::Sync(buf)) + } else { + Ok(Op::Async(Box::new(op))) + } + } + } +} + +pub fn op_write( + _state: &ThreadSafeState, + base: &msg::Base<'_>, + data: Option, +) -> CliOpResult { + let cmd_id = base.cmd_id(); + let inner = base.inner_as_write().unwrap(); + let rid = inner.rid(); + + match resources::lookup(rid) { + None => Err(deno_error::bad_resource()), + Some(resource) => { + let op = tokio_write::write(resource, data.unwrap()) + .map_err(ErrBox::from) + .and_then(move |(_resource, _buf, nwritten)| { + let builder = &mut FlatBufferBuilder::new(); + let inner = msg::WriteRes::create( + builder, + &msg::WriteResArgs { + nbyte: nwritten as u32, + }, + ); + Ok(serialize_response( + cmd_id, + builder, + msg::BaseArgs { + inner: Some(inner.as_union_value()), + inner_type: msg::Any::WriteRes, + ..Default::default() + }, + )) + }); + if base.sync() { + let buf = op.wait()?; + Ok(Op::Sync(buf)) + } else { + Ok(Op::Async(Box::new(op))) + } + } + } +} + pub fn op_seek( _state: &ThreadSafeState, base: &msg::Base<'_>, diff --git a/cli/ops/io.rs b/cli/ops/io.rs deleted file mode 100644 index 610238942..000000000 --- a/cli/ops/io.rs +++ /dev/null @@ -1,43 +0,0 @@ -use super::dispatch_minimal::MinimalOp; -use crate::deno_error; -use crate::resources; -use crate::tokio_write; -use deno::ErrBox; -use deno::PinnedBuf; -use futures::Future; - -pub fn op_read(rid: i32, zero_copy: Option) -> Box { - debug!("read rid={}", rid); - let zero_copy = match zero_copy { - None => { - return Box::new(futures::future::err(deno_error::no_buffer_specified())) - } - Some(buf) => buf, - }; - match resources::lookup(rid as u32) { - None => Box::new(futures::future::err(deno_error::bad_resource())), - Some(resource) => Box::new( - tokio::io::read(resource, zero_copy) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nread)| Ok(nread as i32)), - ), - } -} - -pub fn op_write(rid: i32, zero_copy: Option) -> Box { - debug!("write rid={}", rid); - let zero_copy = match zero_copy { - None => { - return Box::new(futures::future::err(deno_error::no_buffer_specified())) - } - Some(buf) => buf, - }; - match resources::lookup(rid as u32) { - None => Box::new(futures::future::err(deno_error::bad_resource())), - Some(resource) => Box::new( - tokio_write::write(resource, zero_copy) - .map_err(ErrBox::from) - .and_then(move |(_resource, _buf, nwritten)| Ok(nwritten as i32)), - ), - } -} diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index bf621e0e1..021c0fa47 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -1,5 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use crate::deno_error::GetErrorKind; +use crate::dispatch_minimal::dispatch_minimal; +use crate::dispatch_minimal::parse_min_record; use crate::msg; use crate::state::ThreadSafeState; use crate::tokio_util; @@ -11,15 +13,12 @@ use hyper; use hyper::rt::Future; use tokio_threadpool; -mod dispatch_minimal; -mod io; - mod compiler; use compiler::{op_cache, op_fetch_source_file}; mod errors; use errors::{op_apply_source_map, op_format_error}; mod files; -use files::{op_close, op_open, op_seek}; +use files::{op_close, op_open, op_read, op_seek, op_write}; mod fetch; use fetch::op_fetch; mod fs; @@ -72,8 +71,6 @@ fn empty_buf() -> Buf { } const FLATBUFFER_OP_ID: OpId = 44; -const OP_READ: OpId = 1; -const OP_WRITE: OpId = 2; pub fn dispatch_all( state: &ThreadSafeState, @@ -84,18 +81,11 @@ pub fn dispatch_all( ) -> CoreOp { let bytes_sent_control = control.len(); let bytes_sent_zero_copy = zero_copy.as_ref().map(|b| b.len()).unwrap_or(0); - - let op = match op_id { - OP_READ => { - dispatch_minimal::dispatch(io::op_read, state, control, zero_copy) - } - OP_WRITE => { - dispatch_minimal::dispatch(io::op_write, state, control, zero_copy) - } - FLATBUFFER_OP_ID => { - dispatch_all_legacy(state, control, zero_copy, op_selector) - } - _ => panic!("bad op_id"), + let op = if op_id != FLATBUFFER_OP_ID { + let min_record = parse_min_record(control).unwrap(); + dispatch_minimal(state, op_id, min_record, zero_copy) + } else { + dispatch_all_legacy(state, control, zero_copy, op_selector) }; state.metrics_op_dispatched(bytes_sent_control, bytes_sent_zero_copy); op @@ -236,6 +226,7 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { msg::Any::Open => Some(op_open), msg::Any::PermissionRevoke => Some(op_revoke_permission), msg::Any::Permissions => Some(op_permissions), + msg::Any::Read => Some(op_read), msg::Any::ReadDir => Some(op_read_dir), msg::Any::Readlink => Some(op_read_link), msg::Any::Remove => Some(op_remove), @@ -254,6 +245,7 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { msg::Any::Truncate => Some(op_truncate), msg::Any::HomeDir => Some(op_home_dir), msg::Any::Utime => Some(op_utime), + msg::Any::Write => Some(op_write), // TODO(ry) split these out so that only the appropriate Workers can access // them. diff --git a/js/dispatch.ts b/js/dispatch.ts index 6edd2981c..babea5739 100644 --- a/js/dispatch.ts +++ b/js/dispatch.ts @@ -32,13 +32,9 @@ function flatbufferRecordFromBuf(buf: Uint8Array): FlatbufferRecord { } export function handleAsyncMsgFromRust(opId: number, ui8: Uint8Array): void { + const buf32 = new Int32Array(ui8.buffer, ui8.byteOffset, ui8.byteLength / 4); if (opId !== FLATBUFFER_OP_ID) { // Fast and new - const buf32 = new Int32Array( - ui8.buffer, - ui8.byteOffset, - ui8.byteLength / 4 - ); const recordMin = recordFromBufMinimal(opId, buf32); handleAsyncMsgFromRustMinimal(ui8, recordMin); } else { diff --git a/js/dispatch_minimal.ts b/js/dispatch_minimal.ts index 483342127..df0a290b2 100644 --- a/js/dispatch_minimal.ts +++ b/js/dispatch_minimal.ts @@ -52,19 +52,6 @@ export function handleAsyncMsgFromRustMinimal( promise!.resolve(result); } -export function sendSyncMinimal( - opId: number, - arg: number, - zeroCopy: Uint8Array -): number { - scratch32[0] = 0; // promiseId 0 indicates sync - scratch32[1] = arg; - const res = core.dispatch(opId, scratchBytes, zeroCopy)!; - const res32 = new Int32Array(res.buffer, res.byteOffset, 3); - const resRecord = recordFromBufMinimal(opId, res32); - return resRecord.result; -} - export function sendAsyncMinimal( opId: number, arg: number, diff --git a/js/files.ts b/js/files.ts index 1582d8fd4..eb899d738 100644 --- a/js/files.ts +++ b/js/files.ts @@ -11,12 +11,11 @@ import { SyncSeeker } from "./io"; import * as dispatch from "./dispatch"; -import { sendAsyncMinimal, sendSyncMinimal } from "./dispatch_minimal"; +import { sendAsyncMinimal } from "./dispatch_minimal"; import * as msg from "gen/cli/msg_generated"; import { assert } from "./util"; import * as flatbuffers from "./flatbuffers"; -// Warning: These constants defined in two places. Here and in cli/ops/mod.rs. const OP_READ = 1; const OP_WRITE = 2; @@ -63,6 +62,26 @@ export async function open( return resOpen(await dispatch.sendAsync(...reqOpen(filename, mode))); } +function reqRead( + rid: number, + p: Uint8Array +): [flatbuffers.Builder, msg.Any, flatbuffers.Offset, Uint8Array] { + const builder = flatbuffers.createBuilder(); + const inner = msg.Read.createRead(builder, rid); + return [builder, msg.Any.Read, inner, p]; +} + +function resRead(baseRes: null | msg.Base): number | EOF { + assert(baseRes != null); + assert(msg.Any.ReadRes === baseRes!.innerType()); + const res = new msg.ReadRes(); + assert(baseRes!.inner(res) != null); + if (res.eof()) { + return EOF; + } + return res.nread(); +} + /** Read synchronously from a file ID into an array buffer. * * Return `number | EOF` for the operation. @@ -74,14 +93,7 @@ export async function open( * */ export function readSync(rid: number, p: Uint8Array): number | EOF { - const nread = sendSyncMinimal(OP_READ, rid, p); - if (nread < 0) { - throw new Error("read error"); - } else if (nread == 0) { - return EOF; - } else { - return nread; - } + return resRead(dispatch.sendSync(...reqRead(rid, p))); } /** Read from a file ID into an array buffer. @@ -106,6 +118,23 @@ export async function read(rid: number, p: Uint8Array): Promise { } } +function reqWrite( + rid: number, + p: Uint8Array +): [flatbuffers.Builder, msg.Any, flatbuffers.Offset, Uint8Array] { + const builder = flatbuffers.createBuilder(); + const inner = msg.Write.createWrite(builder, rid); + return [builder, msg.Any.Write, inner, p]; +} + +function resWrite(baseRes: null | msg.Base): number { + assert(baseRes != null); + assert(msg.Any.WriteRes === baseRes!.innerType()); + const res = new msg.WriteRes(); + assert(baseRes!.inner(res) != null); + return res.nbyte(); +} + /** Write synchronously to the file ID the contents of the array buffer. * * Resolves with the number of bytes written. @@ -116,12 +145,7 @@ export async function read(rid: number, p: Uint8Array): Promise { * Deno.writeSync(file.rid, data); */ export function writeSync(rid: number, p: Uint8Array): number { - let result = sendSyncMinimal(OP_WRITE, rid, p); - if (result < 0) { - throw new Error("write error"); - } else { - return result; - } + return resWrite(dispatch.sendSync(...reqWrite(rid, p))); } /** Write to the file ID the contents of the array buffer. -- cgit v1.2.3