Make BufWriter/BufWriterSync flush write all chunks

Fixes denoland#6263
marcosc90 · Jun 12, 2020 · 50947b4 · 50947b4
1 parent 1fff6f5
commit 50947b4
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 33 deletions.
diff --git a/std/io/bufio.ts b/std/io/bufio.ts
@@ -426,17 +426,6 @@ abstract class AbstractBufBase {
   buffered(): number {
     return this.usedBufferBytes;
   }
-
-  checkBytesWritten(numBytesWritten: number): void {
-    if (numBytesWritten < this.usedBufferBytes) {
-      if (numBytesWritten > 0) {
-        this.buf.copyWithin(0, numBytesWritten, this.usedBufferBytes);
-        this.usedBufferBytes -= numBytesWritten;
-      }
-      this.err = new Error("Short write");
-      throw this.err;
-    }
-  }
 }
 
 /** BufWriter implements buffering for an deno.Writer object.
@@ -474,18 +463,16 @@ export class BufWriter extends AbstractBufBase implements Writer {
     if (this.err !== null) throw this.err;
     if (this.usedBufferBytes === 0) return;
 
-    let numBytesWritten = 0;
     try {
-      numBytesWritten = await this.writer.write(
+      await Deno.writeAll(
+        this.writer,
         this.buf.subarray(0, this.usedBufferBytes)
       );
     } catch (e) {
       this.err = e;
       throw e;
     }
 
-    this.checkBytesWritten(numBytesWritten);
-
     this.buf = new Uint8Array(this.buf.length);
     this.usedBufferBytes = 0;
   }
@@ -569,18 +556,16 @@ export class BufWriterSync extends AbstractBufBase implements WriterSync {
     if (this.err !== null) throw this.err;
     if (this.usedBufferBytes === 0) return;
 
-    let numBytesWritten = 0;
     try {
-      numBytesWritten = this.writer.writeSync(
+      Deno.writeAllSync(
+        this.writer,
         this.buf.subarray(0, this.usedBufferBytes)
       );
     } catch (e) {
       this.err = e;
       throw e;
     }
 
-    this.checkBytesWritten(numBytesWritten);
-
     this.buf = new Uint8Array(this.buf.length);
     this.usedBufferBytes = 0;
   }

diff --git a/std/io/bufio_test.ts b/std/io/bufio_test.ts
@@ -305,24 +305,24 @@ Deno.test("bufioPeek", async function (): Promise<void> {
   const r = await buf.peek(1);
   assert(r === null);
   /* TODO
-	Test for issue 3022, not exposing a reader's error on a successful Peek.
-	buf = NewReaderSize(dataAndEOFReader("abcd"), 32)
-	if s, err := buf.Peek(2); string(s) != "ab" || err != nil {
-		t.Errorf(`Peek(2) on "abcd", EOF = %q, %v; want "ab", nil`, string(s), err)
-	}
-	if s, err := buf.Peek(4); string(s) != "abcd" || err != nil {
-		t.Errorf(
+  Test for issue 3022, not exposing a reader's error on a successful Peek.
+  buf = NewReaderSize(dataAndEOFReader("abcd"), 32)
+  if s, err := buf.Peek(2); string(s) != "ab" || err != nil {
+    t.Errorf(`Peek(2) on "abcd", EOF = %q, %v; want "ab", nil`, string(s), err)
+  }
+  if s, err := buf.Peek(4); string(s) != "abcd" || err != nil {
+    t.Errorf(
       `Peek(4) on "abcd", EOF = %q, %v; want "abcd", nil`,
       string(s),
       err
     )
-	}
-	if n, err := buf.Read(p[0:5]); string(p[0:n]) != "abcd" || err != nil {
-		t.Fatalf("Read after peek = %q, %v; want abcd, EOF", p[0:n], err)
-	}
-	if n, err := buf.Read(p[0:1]); string(p[0:n]) != "" || err != io.EOF {
-		t.Fatalf(`second Read after peek = %q, %v; want "", EOF`, p[0:n], err)
-	}
+  }
+  if n, err := buf.Read(p[0:5]); string(p[0:n]) != "abcd" || err != nil {
+    t.Fatalf("Read after peek = %q, %v; want abcd, EOF", p[0:n], err)
+  }
+  if n, err := buf.Read(p[0:1]); string(p[0:n]) != "" || err != io.EOF {
+    t.Fatalf(`second Read after peek = %q, %v; want "", EOF`, p[0:n], err)
+  }
   */
 });
 
@@ -497,3 +497,50 @@ Deno.test({
     assertEquals(actual, "hello\nworld\nhow\nare\nyou?\n\nfoobar\n\n");
   },
 });
+
+Deno.test({
+  name: "BufWriter.flush should write all bytes",
+  async fn(): Promise<void> {
+    const bufSize = 16 * 1024;
+    const data = new Uint8Array(bufSize);
+    data.fill("a".charCodeAt(0));
+
+    const writer: Deno.Writer = {
+      cache: [],
+      write(p: Uint8Array): Promise<number> {
+        this.cache.push(p.subarray(0, 1));
+
+        // Writer that only writes 1 byte at a time
+        return Promise.resolve(1);
+      },
+    };
+
+    const bufWriter = new BufWriter(writer);
+    await bufWriter.write(data);
+
+    await bufWriter.flush();
+  },
+});
+
+Deno.test({
+  name: "BufWriterSync.flush should write all bytes",
+  fn(): void {
+    const bufSize = 16 * 1024;
+    const data = new Uint8Array(bufSize);
+    data.fill("a".charCodeAt(0));
+
+    const writer: Deno.WriterSync = {
+      cache: [],
+      writeSync(p: Uint8Array): number {
+        this.cache.push(p.subarray(0, 1));
+        // Writer that only writes 1 byte at a time
+        return 1;
+      },
+    };
+
+    const bufWriter = new BufWriterSync(writer);
+    bufWriter.writeSync(data);
+
+    bufWriter.flush();
+  },
+});