From 50947b4589a405e376afb5ddf62cac89a015c636 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Fri, 12 Jun 2020 21:27:21 +0200 Subject: [PATCH] Make BufWriter/BufWriterSync flush write all chunks Fixes #6263 --- std/io/bufio.ts | 23 +++----------- std/io/bufio_test.ts | 75 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 65 insertions(+), 33 deletions(-) diff --git a/std/io/bufio.ts b/std/io/bufio.ts index e4759dcb7d9ba5..724655735f4bbd 100644 --- 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,9 +463,9 @@ 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) { @@ -484,8 +473,6 @@ export class BufWriter extends AbstractBufBase implements Writer { throw e; } - this.checkBytesWritten(numBytesWritten); - this.buf = new Uint8Array(this.buf.length); this.usedBufferBytes = 0; } @@ -569,9 +556,9 @@ 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) { @@ -579,8 +566,6 @@ export class BufWriterSync extends AbstractBufBase implements WriterSync { 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 index 2a32ba135ad0c7..b7e530bb616151 100644 --- a/std/io/bufio_test.ts +++ b/std/io/bufio_test.ts @@ -305,24 +305,24 @@ Deno.test("bufioPeek", async function (): Promise { 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 { + const bufSize = 16 * 1024; + const data = new Uint8Array(bufSize); + data.fill("a".charCodeAt(0)); + + const writer: Deno.Writer = { + cache: [], + write(p: Uint8Array): Promise { + 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(); + }, +});