A throwing selector during a range insertion leaves a wrong Count on the view
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- Half a day
- Newbie friendliness
- 45/100
Research direction
The bug is in the range branch of the Add handler inside ObservableList<T>.View<TView>. Start with the repro in the issue, which throws from a selector on the second element of AddRange, then check that filteredCount is incremented before the elements reach the view's own list. Done means the view's Count matches its contents after a throwing selector, with a test for that case, or the maintainer documents that selectors must not throw. The issue leaves the choice between those two open, so confirm it before writing a fix.
Written by the indexing model from the issue text.
Description
Summary
When the selector of a view throws for an element of a range insertion, the view of ObservableList<T> counts the elements before it, although it inserts none of them. Count of the view stays wrong from then on.
Repro
var list = new ObservableList<int>();
var view = list.CreateView(x => x == 999 ? throw new InvalidOperationException() : x);
list.AddRange(new[] { 2, 999 }); // throws
// list: 2, 999
// view.Count: 1 - but the view contains no element
Attaching a filter does not change the result. This was observed on master (a78ba19) and on the branch of #125 (7f9dc08).
Cause
In ObservableList<T>.View<TView>, the range branch of the Add handler increments filteredCount for each accepted element while it calls selector and Filter.IsMatch. It inserts the elements into its own list only after the loop. When selector (or the filter) throws for a later element, the earlier increments remain, but no element is inserted.
The exception also leaves the handler of the view, so the handlers of the source after the view miss the change, in the same way as with an exception from a subscriber (#142).
Suggestions
- Compute the new state of the view (views, matches, the new filtered count) before applying any of it, so that an exception from
selectoror the filter leaves the view unchanged. - If that is not wanted, document that selectors and filters must not throw, and that the view is inconsistent after one does.
- Dominant language
- C#
- Stars
- 1k
- Forks
- 73
- Avg merge
- 4d 18h
- Merged PRs (30d)
- 1
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from Cysharp/ObservableCollections
-
Difficulty 4/5 3-5 days Newbie friendliness 55/100
Cysharp/ObservableCollections#146 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Cysharp/ObservableCollections#145 ·
-
AlternateIndexList breaks its ordering or removes the wrong elements without reporting an errorOpen
Difficulty 3/5 1-2 days Newbie friendliness 50/100
Cysharp/ObservableCollections#143 ·
-
Difficulty 4/5 1-2 days Newbie friendliness 48/100
Cysharp/ObservableCollections#142 · 1 comment ·
-
Difficulty 4/5 1-2 days Newbie friendliness 54/100
Cysharp/ObservableCollections#141 · 2 comments ·
All issues in Cysharp/ObservableCollections
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
juandresrodca/AdminForge#54 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
要トリアージ 辞書
Difficulty 1/5 1-3 hours Newbie friendliness 86/100
yksr-melt/Meltype#1625 · 1 reaction ·
Maintainers usually reply within 1 day
-
Docs typoOpen
Difficulty 1/5 Under an hour Newbie friendliness 78/100
MonoGame/docs.monogame.github.io#328 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
space-wizards/RobustToolbox#7197 ·
Maintainers usually reply within 1 day