Avoid attempting to process pending input if we are already processing it

[DavisVaughan]

Addresses #4068

Joint work with @lionel-

#4068 (comment) is a detailed account of the problem that this PR addresses, so definitely give that a close read before trying to understand this.

The reproducible example that is now fixed is #4068 (comment)

The main challenge to deal with is the fact that await this.session.isCodeFragmentComplete(codeFragment) is async and is called in a loop inside processPendingInput(), allowing the "state of the world", particularly this._runtimeItemPendingInput, to change out from under us while we are finding a complete code fragment.

There are a few key changes in this PR

• We introduce a global _pendingInputState to ensure that we cannot re-enter processPendingInput() while we are already processing input

  • If 'Idle', we aren't processing any input
  • If 'Processing', we are processing input and cannot call processPendingInput() again
  • If 'Interrupted', this is a signal that can happen while we are 'Processing' inside processPendingInput() that we should bail out and stop trying to processing any pending input.

• We no longer set this._runtimeItemPendingInput = undefined; in processPendingInput() before all of the await isCodeFragmentComplete() calls. This was particularly bad because at the await point we could loop back around into addPendingInput() and create an entirely new this._runtimeItemPendingInput because it looked like one didn't exist at all!

• We pay very careful attention in processPendingInput() to ensure that we use either this._runtimeItemPendingInput.code or pendingInputLines, both of which are guaranteed to always reflect the current state of the world after the await isCodeFragmentComplete() calls. Previously, the local variable version of pendingInputLines had a strong chance to be outdated after the awaits.

---

This might be a case where it would be useful to write a test that tries to rapidly send some code to the console. If we can come up with a failing test on main (even an intermittent failure would be fine), then we can use that as a test case for this because if that ever fails after this PR then something is wrong.

This is the absolute simplest case I can come up with where mashing Cmd + Enter on each line really really fast will sometimes trigger the issue on main, so that is what I'd try and create a test against.

Sys.sleep(.5)
print("hi there")
Sys.sleep(.5)

x <- 1
y <- x
z <- y
a <- z

[softwarenerd]

LG!

[softwarenerd]

I would make this:

try {
	await this.processPendingInputImpl();
} finally {
	this._pendingInputState = 'Idle';
}

or

try {
	return this.processPendingInputImpl();
} finally {
	this._pendingInputState = 'Idle';
}

[lionel-]

The way I see it:

// Does not do what we want, i.e. does not return the result of the impl helper
await this.processPendingInputImpl();

// Breaks the async / await abstraction by returning a promise instead of a value
return this.processPendingInputImpl();

// Matches the async /await abstraction and correctly returns the result
return await this.processPendingInputImpl();

[DavisVaughan]

@softwarenerd had a question about whether or not we need the Interrupted state here. His point was that maybe we could just check if this._runtimeItemPendingInput == undefined after each await, and if it was, treat that as the interrupt.

I thought about this a little more and determined we do still need it. In theory it is possible for this to happen during a single await:

• clearPendingInput() can be called, setting this._runtimeItemPendingInput == undefined
• addPendingInput() can be called, setting this._runtimeItemPendingInput to a new pending input

In that case, once we get control back we won't be able to tell that we got interrupted if we don't have this 'Interrupted' state that gets fired from clearPendingInput()