eiac/xterm.js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(buffer): avoid shared _extendedAttrs sentinel breaking addon-image

Browse Source 
EMPTY_SPARSE_EXTENDED was shared across lines via copyFrom; addon-image
writes _extendedAttrs directly without copy-on-write, which caused tile
eviction failures and broke instanceof Object checks.

Use undefined for empty extended maps (keep EMPTY_SPARSE_MAP for _combined).
Ensure addon-image forks the map before writes and update the accessor test.

Co-authored-by: Daniel Imms <Tyriar@users.noreply.github.com>
This commit is contained in:
Cursor Agent
2026-06-06 21:51:23 +00:00
co-authored by Daniel Imms
parent 81b3b1ec46
commit dec5452fc0
4 changed files with 38 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/addon-image/src/ImageStorage.ts
+10 -10
View File
@@ -436,9 +436,9 @@ export class ImageStorage implements IDisposable {
for (let col = 0; col < cols; ++col) {
let e: IExtendedAttrsImage;
if (line.getBg(col) & BgFlags.HAS_EXTENDED) {
e = line._extendedAttrs[col] ?? EMPTY_ATTRS;
e = line._extendedAttrs?.[col] ?? EMPTY_ATTRS;
} else {
const maybeImg = line._extendedAttrs[col] as IExtendedAttrsImage | undefined;
const maybeImg = line._extendedAttrs?.[col] as IExtendedAttrsImage | undefined;
if (!maybeImg || maybeImg.imageId === undefined || maybeImg.imageId === -1) {
continue;
}
@@ -461,7 +461,7 @@ export class ImageStorage implements IDisposable {
* Also check _extendedAttrs directly for cells where text cleared HAS_EXTENDED.
*/
while (++col < cols) {
const nextE = line._extendedAttrs[col] as IExtendedAttrsImage | undefined;
const nextE = line._extendedAttrs?.[col] as IExtendedAttrsImage | undefined;
if (!nextE || nextE.imageId !== imageId || nextE.tileId !== startTile + count) {
break;
}
@@ -515,7 +515,7 @@ export class ImageStorage implements IDisposable {
for (let row = 0; row < rows; ++row) {
const line = buffer.lines.get(row) as IBufferLineExt;
if (line.getBg(oldCol) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[oldCol] ?? EMPTY_ATTRS;
const e: IExtendedAttrsImage = line._extendedAttrs?.[oldCol] ?? EMPTY_ATTRS;
const imageId = e.imageId;
if (imageId === undefined || imageId === -1) {
continue;
@@ -560,7 +560,7 @@ export class ImageStorage implements IDisposable {
const buffer = this._terminal._core.buffer;
const line = buffer.lines.get(y) as IBufferLineExt;
if (line && line.getBg(x) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[x] ?? EMPTY_ATTRS;
const e: IExtendedAttrsImage = line._extendedAttrs?.[x] ?? EMPTY_ATTRS;
if (e.imageId && e.imageId !== -1) {
const orig = this._images.get(e.imageId)?.orig;
if (window.ImageBitmap && orig instanceof ImageBitmap) {
@@ -580,7 +580,7 @@ export class ImageStorage implements IDisposable {
const buffer = this._terminal._core.buffer;
const line = buffer.lines.get(y) as IBufferLineExt;
if (line && line.getBg(x) & BgFlags.HAS_EXTENDED) {
const e: IExtendedAttrsImage = line._extendedAttrs[x] ?? EMPTY_ATTRS;
const e: IExtendedAttrsImage = line._extendedAttrs?.[x] ?? EMPTY_ATTRS;
if (e.imageId && e.imageId !== -1 && e.tileId !== -1) {
const spec = this._images.get(e.imageId);
if (spec) {
@@ -611,7 +611,7 @@ export class ImageStorage implements IDisposable {
private _writeToCell(line: IBufferLineExt, x: number, imageId: number, tileId: number): void {
if (line._data[x * Cell.SIZE + Cell.BG] & BgFlags.HAS_EXTENDED) {
const old = line._extendedAttrs[x];
const old = line._extendedAttrs?.[x];
if (old) {
if (old.imageId !== undefined) {
// found an old ExtendedAttrsImage, since we know that
@@ -627,13 +627,13 @@ export class ImageStorage implements IDisposable {
return;
}
// found a plain ExtendedAttrs instance, clone it to new entry
line._extendedAttrs[x] = new ExtendedAttrsImage(old.ext, old.urlId, imageId, tileId);
(line._extendedAttrs ??= {})[x] = new ExtendedAttrsImage(old.ext, old.urlId, imageId, tileId);
return;
}
}
// fall-through: always create new ExtendedAttrsImage entry
line._data[x * Cell.SIZE + Cell.BG] |= BgFlags.HAS_EXTENDED;
line._extendedAttrs[x] = new ExtendedAttrsImage(0, 0, imageId, tileId);
(line._extendedAttrs ??= {})[x] = new ExtendedAttrsImage(0, 0, imageId, tileId);
}
private _evictOnAlternate(): void {
@@ -652,7 +652,7 @@ export class ImageStorage implements IDisposable {
}
for (let x = 0; x < this._terminal.cols; ++x) {
if (line._data[x * Cell.SIZE + Cell.BG] & BgFlags.HAS_EXTENDED) {
const imgId = line._extendedAttrs[x]?.imageId;
const imgId = line._extendedAttrs?.[x]?.imageId;
if (imgId) {
const spec = this._images.get(imgId);
if (spec) {
addons/addon-image/src/Types.ts
+1 -1
View File
@@ -64,7 +64,7 @@ export interface IExtendedAttrsImage extends IExtendedAttrs {
/* eslint-disable */
export interface IBufferLineExt extends IBufferLine {
_extendedAttrs: {[index: number]: IExtendedAttrsImage | undefined};
_extendedAttrs: {[index: number]: IExtendedAttrsImage | undefined} | undefined;
_data: Uint32Array;
}
addons/addon-image/test/ImageAddon.test.ts
+4 -1
View File
@@ -118,7 +118,10 @@ test.describe('ImageAddon', () => {
}
// bufferline privates
strictEqual(await ctx.page.evaluate('window.term._core.buffer.lines.get(0)._data instanceof Uint32Array'), true);
strictEqual(await ctx.page.evaluate('window.term._core.buffer.lines.get(0)._extendedAttrs instanceof Object'), true);
strictEqual(await ctx.page.evaluate(`(() => {
const ea = window.term._core.buffer.lines.get(0)._extendedAttrs;
return ea === undefined || (typeof ea === 'object' && ea !== null);
})()`), true);
// inputhandler privates
strictEqual(await ctx.page.evaluate('window.term._core._inputHandler._curAttrData.constructor.name'), '_AttributeData');
strictEqual(await ctx.page.evaluate('window.term._core._inputHandler._parser.constructor.name'), 'EscapeSequenceParser');
src/common/buffer/BufferLine.ts
+23 -27
View File
@@ -72,20 +72,19 @@ export interface IBufferLineStringCache {
* memory allocs / GC pressure can be greatly reduced by reusing the CellData object.
*/
/**
* Shared empty sparse maps for lines with no combining/extended data.
* Uses a sentinel + copy-on-write (not `undefined`) so `copyFrom` can detect empty maps
* with reference equality. `undefined` was explored but regressed sparse `copyFrom`;
* `Object.freeze` on these sentinels is safe with COW but did not improve perf.
* Shared empty sparse map for lines with no combining data.
* Uses a sentinel + copy-on-write so `copyFrom` can detect empty `_combined` with
* reference equality. `_extendedAttrs` uses `undefined` instead (addon-image writes
* the map directly without BufferLine APIs).
*/
const EMPTY_SPARSE_MAP: {[index: number]: string} = Object.create(null);
const EMPTY_SPARSE_EXTENDED: {[index: number]: IExtendedAttrs | undefined} = Object.create(null);
export class BufferLine implements IBufferLine {
protected _data: Uint32Array;
/** Sparse cache; only read when `IS_COMBINED_MASK` is set in `_data`. */
protected _combined: {[index: number]: string} = EMPTY_SPARSE_MAP;
/** Sparse cache; only read when `HAS_EXTENDED` is set in `_data`. */
protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = EMPTY_SPARSE_EXTENDED;
protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} | undefined;
protected _stringCacheEntryRef: WeakRef<IBufferLineStringCacheEntry> | undefined;
public length: number;
@@ -221,7 +220,7 @@ export class BufferLine implements IBufferLine {
cell.combinedData = '';
}
if (cell.bg & BgFlags.HAS_EXTENDED) {
cell.extended = this._extendedAttrs[index]!;
cell.extended = this._extendedAttrs![index]!;
} else {
// Do not mutate cell.extended in place: it may still reference this line's map entry from a
// prior loadCell into a reused CellData (e.g. $workCell during insert/delete).
@@ -240,8 +239,7 @@ export class BufferLine implements IBufferLine {
this._combined[index] = cell.combinedData;
}
if (cell.bg & BgFlags.HAS_EXTENDED) {
this._ensureExtendedAttrsMap();
this._extendedAttrs[index] = cell.extended;
this._ensureExtendedAttrsMap()[index] = cell.extended;
}
this._data[index * Constants.CELL_INDICIES + Cell.CONTENT] = cell.content;
this._data[index * Constants.CELL_INDICIES + Cell.FG] = cell.fg;
@@ -256,8 +254,7 @@ export class BufferLine implements IBufferLine {
public setCellFromCodepoint(index: number, codePoint: number, width: number, attrs: IAttributeData): void {
this._invalidateStringCache();
if (attrs.bg & BgFlags.HAS_EXTENDED) {
this._ensureExtendedAttrsMap();
this._extendedAttrs[index] = attrs.extended;
this._ensureExtendedAttrsMap()[index] = attrs.extended;
}
this._data[index * Constants.CELL_INDICIES + Cell.CONTENT] = codePoint | (width << Content.WIDTH_SHIFT);
this._data[index * Constants.CELL_INDICIES + Cell.FG] = attrs.fg;
@@ -425,11 +422,13 @@ export class BufferLine implements IBufferLine {
}
}
// remove any cut off extended attributes
const extKeys = Object.keys(this._extendedAttrs);
for (let i = 0; i < extKeys.length; i++) {
const key = parseInt(extKeys[i], 10);
if (key >= cols) {
delete this._extendedAttrs[key];
if (this._extendedAttrs) {
const extKeys = Object.keys(this._extendedAttrs);
for (let i = 0; i < extKeys.length; i++) {
const key = parseInt(extKeys[i], 10);
if (key >= cols) {
delete this._extendedAttrs[key];
}
}
}
}
@@ -466,7 +465,7 @@ export class BufferLine implements IBufferLine {
return;
}
this._combined = EMPTY_SPARSE_MAP;
this._extendedAttrs = EMPTY_SPARSE_EXTENDED;
this._extendedAttrs = undefined;
for (let i = 0; i < this.length; ++i) {
this.setCell(i, fillCellData);
}
@@ -627,8 +626,7 @@ export class BufferLine implements IBufferLine {
this._combined[destCol] = src._combined[srcCol];
}
if (src._data[srcStart + Cell.BG] & BgFlags.HAS_EXTENDED) {
this._ensureExtendedAttrsMap();
this._extendedAttrs[destCol] = src._extendedAttrs[srcCol];
this._ensureExtendedAttrsMap()[destCol] = src._extendedAttrs![srcCol]!;
}
}
@@ -637,10 +635,10 @@ export class BufferLine implements IBufferLine {
const srcCombined = line._combined;
const srcExtended = line._extendedAttrs;
// Fast path: blank / scroll-recycle source (shared empty sentinel or plain empty object).
if (srcCombined === EMPTY_SPARSE_MAP && srcExtended === EMPTY_SPARSE_EXTENDED) {
// Fast path: blank / scroll-recycle source (empty `_combined` sentinel, no extended map).
if (srcCombined === EMPTY_SPARSE_MAP && !srcExtended) {
this._combined = EMPTY_SPARSE_MAP;
this._extendedAttrs = EMPTY_SPARSE_EXTENDED;
this._extendedAttrs = undefined;
return;
}
this._copySparseMapsFromDense(line);
@@ -663,7 +661,7 @@ export class BufferLine implements IBufferLine {
combined[i] = srcCombined[i];
}
if (srcData[start + Cell.BG] & BgFlags.HAS_EXTENDED) {
extendedAttrs[i] = srcExtended[i];
extendedAttrs[i] = srcExtended![i];
}
}
}
@@ -674,9 +672,7 @@ export class BufferLine implements IBufferLine {
}
}
private _ensureExtendedAttrsMap(): void {
if (this._extendedAttrs === EMPTY_SPARSE_EXTENDED) {
this._extendedAttrs = {};
}
private _ensureExtendedAttrsMap(): {[index: number]: IExtendedAttrs | undefined} {
return this._extendedAttrs ??= {};
}
}