Fix crash when switching converter categories#2483
Closed
veler wants to merge 1 commit into
Closed
Conversation
Fix crash when switching converter categories
Navigating between Converter categories (e.g. Currency -> Temperature,
Temperature -> Weight and mass) intermittently crashed the app.
Root cause: on a category change UnitConverterViewModel rebuilds the bound
Units collection in place and then assigns Unit1/Unit2, which x:Bind pushes to
each ComboBox's SelectedItem synchronously inside the NavigationView selection
handler. WinUI's ComboBox validates SelectedItem against its own item
collection, which it refreshes from the ItemsSource change on a later, deferred
pass. When the selection is applied before that refresh completes,
Selector.put_SelectedItem throws ArgumentException (0x80070057) which, being
unhandled in the XAML delegate, fail-fasts the process. It's timing-dependent,
so it reproduces intermittently and vanishes under a debugger.
The C++/CX version never hit this because classic {Binding} routed selection
through the AlwaysSelectedCollectionView (ICollectionView) currency, which
defers gracefully. That currency integration does not reproduce in the C#
projection: reverting to {Binding} makes the ComboBox write its default (index 0)
selection back through the TwoWay binding, corrupting the engine's from/to
units instead. SelectedIndex binding, a single Reset notification, an atomic
collection swap, and a deferred dispatcher push were all verified on-device not
to close the race, because the lag lives inside the control.
Contain the specific transient at the assignment site (AssignSelectedUnit):
the Unit1/Unit2 backing fields are set before the throw and the ComboBox
reconciles its selection on the next layout pass, so the displayed selection is
correct. Verified: 0 crashes across 110+ category navigations; conversions
(e.g. 100 C = 212 F) and dropdown selection continue to work.
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.
Fix crash when switching converter categories
Navigating between Converter categories (e.g. Currency -> Temperature, Temperature -> Weight and mass) intermittently crashed the app.
Root cause: on a category change UnitConverterViewModel rebuilds the bound Units collection in place and then assigns Unit1/Unit2, which x:Bind pushes to each ComboBox's SelectedItem synchronously inside the NavigationView selection handler. WinUI's ComboBox validates SelectedItem against its own item collection, which it refreshes from the ItemsSource change on a later, deferred pass. When the selection is applied before that refresh completes, Selector.put_SelectedItem throws ArgumentException (0x80070057) which, being unhandled in the XAML delegate, fail-fasts the process. It's timing-dependent, so it reproduces intermittently and vanishes under a debugger.
The C++/CX version never hit this because classic {Binding} routed selection through the AlwaysSelectedCollectionView (ICollectionView) currency, which defers gracefully. That currency integration does not reproduce in the C# projection: reverting to {Binding} makes the ComboBox write its default (index 0) selection back through the TwoWay binding, corrupting the engine's from/to units instead. SelectedIndex binding, a single Reset notification, an atomic collection swap, and a deferred dispatcher push were all verified on-device not to close the race, because the lag lives inside the control.
Contain the specific transient at the assignment site (AssignSelectedUnit): the Unit1/Unit2 backing fields are set before the throw and the ComboBox reconciles its selection on the next layout pass, so the displayed selection is correct. Verified: 0 crashes across 110+ category navigations; conversions (e.g. 100 C = 212 F) and dropdown selection continue to work.