Revert "Remove dead code: legacy read/write ops"

This is causing a segfault for unknown reasons - see #2787.

This reverts commit 498f6ad431.

cli/dispatch_minimal.rs

@@ -7,11 +7,14 @@
 use crate::state::ThreadSafeState;
 use deno::Buf;
 use deno::CoreOp;
-use deno::ErrBox;
 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 {
@@ -69,23 +72,21 @@ fn test_parse_min_record() {
   assert_eq!(parse_min_record(&buf), None);
 }
 
-pub type MinimalOp = dyn Future<Item = i32, Error = ErrBox> + Send;
-pub type Dispatcher = fn(i32, Option<PinnedBuf>) -> Box<MinimalOp>;
-
-pub fn dispatch(
-  d: Dispatcher,
+pub fn dispatch_minimal(
   state: &ThreadSafeState,
-  control: &[u8],
+  op_id: OpId,
+  mut record: Record,
   zero_copy: Option<PinnedBuf>,
 ) -> CoreOp {
-  let mut record = parse_min_record(control).unwrap();
   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 rid = record.arg;
-  let min_op = d(rid, zero_copy);
-
   let fut = Box::new(min_op.then(move |result| -> Result<Buf, ()> {
     match result {
       Ok(r) => {
@@ -108,3 +109,54 @@ pub fn dispatch(
     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<Item = i32, Error = ErrBox> + Send;
+
+  pub fn read(rid: i32, zero_copy: Option<PinnedBuf>) -> Box<MinimalOp> {
+    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<PinnedBuf>) -> Box<MinimalOp> {
+    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)),
+      ),
+    }
+  }
+}

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;

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;
 }

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<PinnedBuf>,
+) -> 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<PinnedBuf>,
+) -> 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<'_>,

cli/ops/io.rs

@@ -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<PinnedBuf>) -> Box<MinimalOp> {
-  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<PinnedBuf>) -> Box<MinimalOp> {
-  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)),
-    ),
-  }
-}

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<CliDispatchFn> {
     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<CliDispatchFn> {
     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.

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 {

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,

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<number | EOF> {
   }
 }
 
+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<number | EOF> {
  *       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.