TypeError: Cannot read properties of undefined (reading '31')

[SimonSiefke]

Details

• Browser and browser version: VSCode
• OS version: Ubuntu 23.04
• xterm.js version: 5.2.0-beta.35

Steps to reproduce

I'm not quite sure of any consistent reproduction steps, but

1. I was using the VSCode terminal as usual
2. noticed high cpu load and then
3. opened devtools and saw this error (~250 times)

workbench.js:2699 [uncaught exception]: TypeError: Cannot read properties of undefined (reading '31')
workbench.js:2702 TypeError: Cannot read properties of undefined (reading '31')
    at g._setCellUnderline (DomRenderer.ts:388:33)
    at g._handleLinkLeave (DomRenderer.ts:384:10)
    at DomRenderer.ts:82:66
    at t.EventEmitter.fire (EventEmitter.ts:55:16)
    at c._fireUnderlineEvent (Linkifier2.ts:368:13)
    at c._linkLeave (Linkifier2.ts:375:14)
    at c._clearCurrentLink (Linkifier2.ts:263:12)
    at c._handleHover (Linkifier2.ts:115:12)
    at c._handleMouseMove (Linkifier2.ts:106:12)

Additional Information

VSCode issue: microsoft/vscode#184255

XTermjs code:

xterm.js/src/browser/renderer/dom/DomRenderer.ts

Lines 395 to 397 in 118915c

	private _setCellUnderline(x: number, x2: number, y: number, y2: number, cols: number, enabled: boolean): void {
	x = this._cellToRowElements[y][x];
	x2 = this._cellToRowElements[y2][x2];

There might be an edge case in this code when y or y2 is larger than _cellToRowElements.length, then a TypeError is thrown. Another possibility could be that for some reason y or y2 is NaN . But it's still unclear to me why/when this would happen.

[jerch]

Hmm, yes thats prolly related to the double index access, where y|y2 might be out of range. It would be really good, if you can find a repro, as this should never have happened (thus there is most likely already something off at higher level).

A quickfix here might be to test on correct lengths beforehand, so index access wont fail.

Edit: Oh well, lookin at the code again reveals, that the event brings in its own cols value. Ofc this might not be in sync anymore with current terminal state, if a resize happened in between. Looks pretty much like a race condition between terminal resize and event handler execution of _handleLinkLeave.

Edit2:
@Tyriar - Does a resize trigger a screen refresh immediately? Or will this get done by the next setTimeout cycle? If its the latter, then y|y2 or _cellToRowElements[x|x2] is prolly out of sync compared to term.cols|rows (as the event handler might get called before the rendering refreshed the data helpers).

[jerch]

@SimonSiefke Can repro it, still not sure though, when exactly it happens.

[jerch]

Repro:

• open demo
• disable canvas and webgl renderer
• under "Test" click "test URLs"
• click on "Options" and click into "cols"
• move mouse over terminal output up & down to trigger _handleLinkHover and _handleLinkLeave over and over
• keep moving the mouse, now also shrink the terminal output by pressing down arrow --> and the error will eventually occur

Further debugging reveals, that the _handleLinkLeave call receives negative values for y or y2 😱. As a quickfix we can just exit early for negative values. Still we prolly should check, why the linkifier event sends negative y values in the first place.

[jerch]

@SimonSiefke Thx for finding the issue, its fixed with #4546.

[Tyriar]

@Tyriar - Does a resize trigger a screen refresh immediately? Or will this get done by the next setTimeout cycle? If its the latter, then y|y2 or _cellToRowElements[x|x2] is prolly out of sync compared to term.cols|rows (as the event handler might get called before the rendering refreshed the data helpers).

I think all renders are async via the next animation frame, this is why on some machines canvas/webgl may flicker an empty terminal as resize triggers an immediate resize of the canvas which clears the texture.