Skip to content

Commit 9d1291c

Browse files
committed
fix(window): crash on rapid resize, atomic position+size update, bounds guards
- BorderRenderer: null guards and width snapshot to prevent cache race during rapid resize (fixes NullReferenceException in DrawVisibleBorders) - Window: add SetPositionAndSize() for atomic position+size updates, prevents render thread from seeing partial state during drag/resize - WindowPositioningManager: use SetPositionAndSize() in ResizeWindowTo - ConsoleWindowSystem: fix hardcoded Max(1) to Max(0) in screen resize handler, guard DesktopBottomRight/DesktopDimensions against negative values when panels exceed screen height
1 parent 4cef820 commit 9d1291c

4 files changed

Lines changed: 57 additions & 15 deletions

File tree

SharpConsoleUI/ConsoleWindowSystem.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -460,12 +460,16 @@ public event EventHandler<Core.IdleStateEventArgs>? IdleStateChanged
460460
/// <summary>
461461
/// Gets the bottom-right coordinate of the usable desktop area (excluding status bars).
462462
/// </summary>
463-
public Point DesktopBottomRight => new Point(_consoleDriver.ScreenSize.Width - 1, _consoleDriver.ScreenSize.Height - 1 - Render.GetTopStatusHeight() - Render.GetBottomStatusHeight());
463+
public Point DesktopBottomRight => new Point(
464+
_consoleDriver.ScreenSize.Width - 1,
465+
Math.Max(Render.GetTopStatusHeight(), _consoleDriver.ScreenSize.Height - 1 - Render.GetTopStatusHeight() - Render.GetBottomStatusHeight()));
464466

465467
/// <summary>
466468
/// Gets the dimensions of the usable desktop area (excluding status bars).
467469
/// </summary>
468-
public Helpers.Size DesktopDimensions => new Helpers.Size(_consoleDriver.ScreenSize.Width, _consoleDriver.ScreenSize.Height - Render.GetTopStatusHeight() - Render.GetBottomStatusHeight());
470+
public Helpers.Size DesktopDimensions => new Helpers.Size(
471+
_consoleDriver.ScreenSize.Width,
472+
Math.Max(0, _consoleDriver.ScreenSize.Height - Render.GetTopStatusHeight() - Render.GetBottomStatusHeight()));
469473

470474
/// <summary>
471475
/// Gets the visible regions manager for calculating window visibility.
@@ -841,7 +845,7 @@ private void HandleScreenResize(object? sender, Size size)
841845
}
842846
if (window.Top + window.Height > desktopSize.Height)
843847
{
844-
window.Top = Math.Max(1, desktopSize.Height - window.Height);
848+
window.Top = Math.Max(0, desktopSize.Height - window.Height);
845849
}
846850
}
847851

SharpConsoleUI/Window.State.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,38 @@ internal void SetPositionDirect(Point point)
553553
_top = point.Y;
554554
}
555555

556+
/// <summary>
557+
/// Sets position and size atomically, with a single invalidation pass.
558+
/// Prevents the render thread from seeing partial state (old position with new size).
559+
/// </summary>
560+
internal void SetPositionAndSize(Point position, int width, int height)
561+
{
562+
// Apply constraints
563+
if (_minimumWidth != null && width < _minimumWidth)
564+
width = (int)_minimumWidth;
565+
if (_minimumHeight != null && height < _minimumHeight)
566+
height = (int)_minimumHeight;
567+
568+
bool changed = _left != position.X || _top != position.Y ||
569+
_width != width || _height != height;
570+
if (!changed) return;
571+
572+
// Update all fields together
573+
_left = position.X;
574+
_top = position.Y;
575+
_width = width;
576+
_height = height;
577+
578+
// Single invalidation pass
579+
InvalidateBorderCache();
580+
Invalidate(true);
581+
582+
if (_scrollOffset > ContentLineCount - (Height - 2))
583+
{
584+
GoToBottom();
585+
}
586+
}
587+
556588
/// <summary>
557589
/// Sets the window size with proper invalidation and constraint handling.
558590
/// </summary>

SharpConsoleUI/Windows/BorderRenderer.cs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -253,45 +253,52 @@ private void DrawVisibleBorders(List<Rectangle> visibleRegions)
253253
var bg = _window.BackgroundColor;
254254

255255
// Rebuild cached border buffers if necessary
256+
// Snapshot width to avoid race with resize changing _window.Width between check and use
257+
var windowWidth = _window.Width;
256258
if (_cachedTopBorder == null ||
257-
_cachedBorderWidth != _window.Width ||
259+
_cachedBorderWidth != windowWidth ||
258260
_cachedBorderIsActive != isActive)
259261
{
260262
_cachedTopBorder = BuildTopBorder(chars, borderFg, titleFg, buttonFg, closeFg, bg);
261263
_cachedBottomBorder = BuildBottomBorder(chars, borderFg, bg);
262-
_cachedBorderWidth = _window.Width;
264+
_cachedBorderWidth = windowWidth;
263265
_cachedBorderIsActive = isActive;
264266
}
265267

266268
var contentHeight = _window.TotalLines;
267269
var visibleHeight = _window.Height - 2;
268270
var scrollbarVisible = _window.IsScrollable && contentHeight > visibleHeight;
271+
var cachedTop = _cachedTopBorder;
272+
var cachedBottom = _cachedBottomBorder;
269273

270274
foreach (var region in visibleRegions ?? new List<Rectangle>())
271275
{
272276
// Top border
273-
if (region.Top == _window.Top)
277+
if (region.Top == _window.Top && cachedTop != null)
274278
{
275279
int borderStartX = Math.Max(region.Left, _window.Left);
276-
int borderWidth = Math.Min(region.Width, _window.Left + _window.Width - borderStartX);
280+
int borderWidth = Math.Min(region.Width, _window.Left + windowWidth - borderStartX);
281+
// Clamp to cached buffer width in case of resize race
282+
borderWidth = Math.Min(borderWidth, cachedTop.Width - Math.Max(0, borderStartX - _window.Left));
277283
if (borderWidth > 0)
278284
{
279285
int srcX = borderStartX - _window.Left;
280286
driver.WriteBufferRegion(borderStartX, region.Top + desktopUpperLeft.Y,
281-
_cachedTopBorder, srcX, 0, borderWidth, bg);
287+
cachedTop, srcX, 0, borderWidth, bg);
282288
}
283289
}
284290

285291
// Bottom border
286-
if (region.Top + region.Height == _window.Top + _window.Height)
292+
if (region.Top + region.Height == _window.Top + _window.Height && cachedBottom != null)
287293
{
288294
int borderStartX = Math.Max(region.Left, _window.Left);
289-
int borderWidth = Math.Min(region.Width, _window.Left + _window.Width - borderStartX);
295+
int borderWidth = Math.Min(region.Width, _window.Left + windowWidth - borderStartX);
296+
borderWidth = Math.Min(borderWidth, cachedBottom.Width - Math.Max(0, borderStartX - _window.Left));
290297
if (borderWidth > 0)
291298
{
292299
int srcX = borderStartX - _window.Left;
293300
driver.WriteBufferRegion(borderStartX, _window.Top + _window.Height - 1 + desktopUpperLeft.Y,
294-
_cachedBottomBorder!, srcX, 0, borderWidth, bg);
301+
cachedBottom, srcX, 0, borderWidth, bg);
295302
}
296303
}
297304
}

SharpConsoleUI/Windows/WindowPositioningManager.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,9 @@ public void ResizeWindowTo(Window window, int newLeft, int newTop, int newWidth,
123123
// Add old bounds to pending clears
124124
_renderCoordinator.AddPendingDesktopClear(oldBounds);
125125

126-
// Apply the new position and size
127-
window.SetPositionDirect(new Point(newLeft, newTop));
128-
window.SetSize(newWidth, newHeight);
129-
window.Invalidate(true);
126+
// Apply position and size atomically (prevents render thread from
127+
// seeing partial state between position and size changes)
128+
window.SetPositionAndSize(new Point(newLeft, newTop), newWidth, newHeight);
130129

131130
// Invalidate windows that were underneath (now exposed)
132131
InvalidateExposedRegions(window, oldBounds);

0 commit comments

Comments
 (0)