Relax ordering restriction for eval ids

Summary:
Here is a correct scenario in which evaluation requests can be completed "out of order":

1. eval request #1 is sent, push #1 onto stack
2. eval request #2 is sent, push #2 onto stack
3. response for #1 comes back from HHVM, pop #2 off stack
4. #1 != #2 so the invariant is triggered

So we can be more lax about the expected order that we receive responses.  In general, there's no reason to impose any restrictions on the ordering in which requests and responses happen, so let's just use a set.

Changelog: Fix for PHP debugger evaluation crashes.

Reviewed By: yinghuitan

Differential Revision: D4282772

fbshipit-source-id: 7d6169fa660d489b06b1a28e07140fa11a5464e4

pkg/nuclide-debugger-php-rpc/lib/DbgpSocket.js

@@ -134,7 +134,7 @@ export class DbgpSocket {
   _emitter: EventEmitter;
   _isClosed: boolean;
   _messageHandler: DbgpMessageHandler;
-  _pendingEvalTransactionIdStack: Array<number>;
+  _pendingEvalTransactionIds: Set<number>;
   _lastContinuationCommandTransactionId: ?number;
 
   constructor(socket: Socket) {
@@ -144,7 +144,7 @@ export class DbgpSocket {
     this._emitter = new EventEmitter();
     this._isClosed = false;
     this._messageHandler = getDbgpMessageHandlerInstance();
-    this._pendingEvalTransactionIdStack = [];
+    this._pendingEvalTransactionIds = new Set();
     this._lastContinuationCommandTransactionId = null;
 
     socket.on('end', this._onEnd.bind(this));
@@ -230,20 +230,20 @@ export class DbgpSocket {
   }
 
   _handleEvaluationCommand(transactionId: number, message: string): void {
-    invariant(this._pendingEvalTransactionIdStack.length > 0, 'No pending Eval Ids');
-    const lastEvalId = this._pendingEvalTransactionIdStack.pop();
+    invariant(this._pendingEvalTransactionIds.size > 0, 'No pending Eval Ids');
+    invariant(
+      this._pendingEvalTransactionIds.has(transactionId),
+      'Got evaluation response for a request that was never sent.',
+    );
+    this._pendingEvalTransactionIds.delete(transactionId);
     const continuationId = this._lastContinuationCommandTransactionId;
     if (continuationId == null) {
       return;
     }
     // In this case, we are processing the second response to our eval request.  So we can
     // complete the current continuation command promise, and then complete the original
     // eval command promise.
-    invariant(
-      lastEvalId === transactionId,
-      'Evaluation requests are being processed out of order.',
-    );
-    if (this._pendingEvalTransactionIdStack.length === 0) {
+    if (this._pendingEvalTransactionIds.size === 0) {
       // This is the last eval command before returning to the dummy connection entry-point, so
       // we will signal to the CM that the dummy connection is now un-viewable.
       this._emitStatus(ConnectionStatus.DummyIsHidden);
@@ -521,7 +521,7 @@ export class DbgpSocket {
   ): Promise<Object> {
     const transactionId = this._sendCommand(command, params);
     if (isEvaluationCommand(command)) {
-      this._pendingEvalTransactionIdStack.push(transactionId);
+      this._pendingEvalTransactionIds.add(transactionId);
     }
     const isContinuation = isContinuationCommand(command);
     if (isContinuation) {

pkg/nuclide-debugger-php-rpc/spec/DbgpSocket-spec.js

@@ -170,7 +170,7 @@ describe('debugger-php-rpc DbgpSocket', () => {
 
   it('runtimeEvaluate - continuation from eval with break', () => {
     waitsForPromise(async () => {
-      expect(dbgpSocket._pendingEvalTransactionIdStack.length).toBe(0);
+      expect(dbgpSocket._pendingEvalTransactionIds.size).toBe(0);
       dbgpSocket.runtimeEvaluate('foo()');
       testCallResult(
         'eval -i 1 -- Zm9vKCk=',
@@ -180,7 +180,7 @@ describe('debugger-php-rpc DbgpSocket', () => {
           transaction_id: '1',
         },
       );
-      expect(dbgpSocket._pendingEvalTransactionIdStack[0]).toBe(1);
+      expect([...dbgpSocket._pendingEvalTransactionIds][0]).toBe(1);
       dbgpSocket.sendContinuationCommand(COMMAND_RUN);
       expect(onStatus).toHaveBeenCalledWith(ConnectionStatus.Running);
       await testCallResult(
@@ -191,7 +191,7 @@ describe('debugger-php-rpc DbgpSocket', () => {
         },
       );
       expect(dbgpSocket._lastContinuationCommandTransactionId).toBe(null);
-      expect(dbgpSocket._pendingEvalTransactionIdStack.length).toBe(0);
+      expect(dbgpSocket._pendingEvalTransactionIds.size).toBe(0);
     });
   });