From 8b68cdd73684be6da7e8076d38c5b0eb4496cd41 Mon Sep 17 00:00:00 2001
From: Guangcong Luo <guangcongluo@gmail.com>
Date: Sun, 1 Nov 2020 01:51:12 +0000
Subject: [PATCH] Fix Streams bug

It turns out 001f98b4f2ba was wrong.

When urkerab asked why it `peek` wasn't awaited:

https://github.com/smogon/pokemon-showdown/commit/e91c4c5260407e40f24a0662a16d9a3a8118ab37#commitcomment-41364837

The answer was because clearing the buffer after peeking needed to
happen synchronous: if the buffer is written to after peeking but
before the buffer is cleared, that write is lost forever.

This just goes to show, if you do something subtle enough to require
type assertions, you should probably add a comment about what's going
on.

Fixes #7605

This also removes `BattleStream#start()` which is completely useless
API complication. A better implementation would properly forward
crashes between streams (maybe `pipeTo` should do this) but as it
stands, it's not doing anything.
---
 lib/streams.ts       | 13 +++++++++++--
 pokemon-showdown     | 11 +++++------
 sim/battle-stream.ts |  3 ++-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/lib/streams.ts b/lib/streams.ts
index 9d4d293f5f..4e225a320c 100644
--- a/lib/streams.ts
+++ b/lib/streams.ts
@@ -249,7 +249,12 @@ export class ReadStream {
 			byteCount = null;
 		}
 		await this.loadIntoBuffer(byteCount, true);
-		const out = await this.peek(byteCount, encoding);
+
+		// This MUST NOT be awaited: we MUST synchronously clear byteCount after peeking
+		// if the buffer is written to after peek but before clearing the buffer, the write
+		// will be lost forever
+		const out = this.peek(byteCount, encoding) as string | null;
+
 		if (byteCount === null || byteCount >= this.bufSize) {
 			this.bufStart = 0;
 			this.bufEnd = 0;
@@ -299,7 +304,11 @@ export class ReadStream {
 
 	async readBuffer(byteCount: number | null = null) {
 		await this.loadIntoBuffer(byteCount, true);
-		const out = await this.peekBuffer(byteCount);
+
+		// This MUST NOT be awaited: we must synchronously clear the buffer after reading
+		// (see `read`)
+		const out = this.peekBuffer(byteCount) as Buffer | null;
+
 		if (byteCount === null || byteCount >= this.bufSize) {
 			this.bufStart = 0;
 			this.bufEnd = 0;
diff --git a/pokemon-showdown b/pokemon-showdown
index 4c2791c13f..87557ff44d 100755
--- a/pokemon-showdown
+++ b/pokemon-showdown
@@ -109,7 +109,7 @@ if (!process.argv[2] || /^[0-9]+$/.test(process.argv[2])) {
 			var TeamValidator = require('./.sim-dist/team-validator').TeamValidator;
 			var validator = TeamValidator.get(process.argv[3]);
 			var Streams = require('./.lib-dist/streams');
-			var stdin = new Streams.ReadStream(process.stdin);
+			var stdin = Streams.stdin();
 
 			stdin.readLine().then(function (textTeam) {
 				try {
@@ -131,8 +131,8 @@ if (!process.argv[2] || /^[0-9]+$/.test(process.argv[2])) {
 		{
 			var BattleTextStream = require('./.sim-dist/battle-stream').BattleTextStream;
 			var Streams = require('./.lib-dist/streams');
-			var stdin = new Streams.ReadStream(process.stdin);
-			var stdout = new Streams.WriteStream(process.stdout);
+			var stdin = Streams.stdin();
+			var stdout = Streams.stdout();
 
 			var args = process.argv.slice(3);
 
@@ -173,7 +173,6 @@ if (!process.argv[2] || /^[0-9]+$/.test(process.argv[2])) {
 				debug: debug,
 				replay: spectate ? 'spectator' : replay,
 			});
-			battleStream.start();
 			stdin.pipeTo(battleStream);
 			battleStream.pipeTo(stdout);
 		}
@@ -182,7 +181,7 @@ if (!process.argv[2] || /^[0-9]+$/.test(process.argv[2])) {
 		{
 			var Dex = require('./.sim-dist/dex').Dex;
 			var Streams = require('./.lib-dist/streams');
-			var stdin = new Streams.ReadStream(process.stdin);
+			var stdin = Streams.stdin();
 
 			stdin.readLine().then(function (packedTeam) {
 				try {
@@ -200,7 +199,7 @@ if (!process.argv[2] || /^[0-9]+$/.test(process.argv[2])) {
 		{
 			var Dex = require('./.sim-dist/dex').Dex;
 			var Streams = require('./.lib-dist/streams');
-			var stdin = new Streams.ReadStream(process.stdin);
+			var stdin = Streams.stdin();
 
 			stdin.readLine().then(function (unpackedTeam) {
 				try {
diff --git a/sim/battle-stream.ts b/sim/battle-stream.ts
index c5fc399a19..73f143136f 100644
--- a/sim/battle-stream.ts
+++ b/sim/battle-stream.ts
@@ -261,9 +261,10 @@ export class BattleTextStream extends Streams.ReadWriteStream {
 		super();
 		this.battleStream = new BattleStream(options);
 		this.currentMessage = '';
+		void this._listen();
 	}
 
-	async start() {
+	async _listen() {
 		for await (let message of this.battleStream) {
 			if (!message.endsWith('\n')) message += '\n';
 			this.push(message + '\n');