Fix Carousel layout crashes from missing null/bounds checks in AdaptiveCarouselRenderer#9375
Open
karan68 wants to merge 1 commit into
Open
Fix Carousel layout crashes from missing null/bounds checks in AdaptiveCarouselRenderer#9375karan68 wants to merge 1 commit into
karan68 wants to merge 1 commit into
Conversation
Fix multiple crash paths in AdaptiveCarouselRenderer that cause STOWED_EXCEPTION (0x88000fa8) during XAML layout pass when rendering Carousel elements. Watson bucket: 39c24f3b-446a-5167-cd1a-fbaacb663624 (7,923 hits, primarily WidgetBoard 77.6%). Fixes: 1. SelectionChanged handler: FlipView fires SelectionChanged with SelectedIndex=-1 during Items().Append(). Calling pipsPager.SelectedPageIndex(-1) throws InvalidOperationException. Added early return when val < 0. 2. SetFlipViewMaxHeight: GetAt(selectIndex) called without validating selectIndex bounds. SelectedIndex() can be -1 when no item is selected. Added bounds check before GetAt(). 3. SetFlipViewMaxHeight: try_as<FrameworkElement>() result used without null check before calling .Measure(). Added null guard. 4. Render loop: try_as<FrameworkElement>() chained directly into .LayoutUpdated() without null check. Stored result and guarded. 5. InitialPage: GetUInt32() used as SelectedIndex without validating against Items().Size(). Added bounds check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes multiple crash paths in
AdaptiveCarouselRendererthat causeSTOWED_EXCEPTION(0x88000fa8) during XAML layout pass when rendering Carousel elements.Affected apps: WidgetBoard (77.6%), lockapp (3.9%)
Affected SDK:
AdaptiveCards.Rendering.Uwp 3.2.x,AdaptiveCards.Rendering.WinUI3 2.2.3-betaRoot Cause
AdaptiveCarouselRenderer.cpphas several unguarded code paths that throw during XAML layout/rendering:Bug 1 — SelectionChanged handler crashes with SelectedIndex = -1
When
carouselUI.Items().Append()is called during rendering, the FlipView firesSelectionChangedbefore any item is selected (SelectedIndex() == -1). The handler callspipsPager.SelectedPageIndex(-1)which throwsInvalidOperationExceptionbecause PipsPager does not accept negative indices.Bug 2 —
SetFlipViewMaxHeightcallsGetAt()without bounds checkflipView.SelectedIndex()can return -1 (no selection).GetAt(-1)throwsE_BOUNDS. This function is called from aLayoutUpdatedevent handler — outside thetry-catchinRender()— so the exception propagates unhandled and becomes a stowed exception that crashes the host process.Bug 3 —
SetFlipViewMaxHeightcalls.Measure()on uncheckedtry_asresulttry_as<FrameworkElement>()can returnnullptr. The code calls.Measure()on the result without a null check → null pointer dereference.Bug 4 —
try_as<FrameworkElement>()chained directly into.LayoutUpdated()In the render loop,
carouselPageUI.try_as<FrameworkElement>().LayoutUpdated(...)chains the method call without checking iftry_asreturned null.Bug 5 —
InitialPageindex used without bounds validationcarousel.InitialPage().GetUInt32()is passed directly toSelectedIndex()without checking it's withinItems().Size().Fix
SelectedIndex() < 0selectIndex >= 0 && < Items().Size()beforeGetAt(). Null-checktry_asresult before.Measure().try_asresult and guard.LayoutUpdated()call with null check.carouselUI.Items().Size()before setting.Repro
source/uwp/winui3/SimpleVisualizer/SimpleVisualizer.slnClipchampCarousel.jsonSystem.InvalidOperationException: 'Operation is not valid due to the current state of the object.'duringRenderAdaptiveCard()Files Changed
source/uwp/SharedRenderer/lib/AdaptiveCarouselRenderer.cpp—> All 5 fixessource/uwp/winui3/SimpleVisualizer/ClipchampCarousel.json—> Test template (Clipchamp-style Carousel with empty Container)source/uwp/winui3/SimpleVisualizer/MainWindow.xaml.cs—> UseAppContext.BaseDirectoryinstead ofPackage.Current.InstalledLocation.Path