Fix Streams bug

It turns out 001f98b4f2 was wrong.

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

e91c4c5260 (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.
This commit is contained in:
Guangcong Luo 2020-11-01 01:51:12 +00:00
parent 97ed534d64
commit 8b68cdd736
3 changed files with 18 additions and 9 deletions

View File

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

View File

@ -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 {

View File

@ -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');