From 3f710108f2d7a67ff460c058a6f6bd532d428654 Mon Sep 17 00:00:00 2001 From: Marcos Casagrande Date: Thu, 25 Jun 2020 14:17:33 +0200 Subject: [PATCH] fix(std/io): Make BufWriter/BufWriterSync.flush write all chunks (#6269) --- std/io/bufio.ts | 23 +++--------- std/io/bufio_test.ts | 83 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 73 insertions(+), 33 deletions(-) diff --git a/std/io/bufio.ts b/std/io/bufio.ts index cd25736830..a1e6f67aa0 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 6bfd2cff90..ad02703de8 100644 --- a/std/io/bufio_test.ts +++ b/std/io/bufio_test.ts @@ -306,24 +306,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) + } */ }); @@ -498,3 +498,58 @@ 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 cache: Uint8Array[] = []; + const writer: Deno.Writer = { + write(p: Uint8Array): Promise { + 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(); + const buf = new Uint8Array(cache.length); + for (let i = 0; i < cache.length; i++) buf.set(cache[i], i); + + assertEquals(data, buf); + }, +}); + +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 cache: Uint8Array[] = []; + const writer: Deno.WriterSync = { + writeSync(p: Uint8Array): number { + 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(); + const buf = new Uint8Array(cache.length); + for (let i = 0; i < cache.length; i++) buf.set(cache[i], i); + + assertEquals(data, buf); + }, +});