Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing IndexOutOfRangeException throwing by DGV when disposing its DataSource #4551

Conversation

vladimir-krestov
Copy link
Contributor

@vladimir-krestov vladimir-krestov commented Feb 10, 2021

Fixes #4216

Proposed changes

  • Add Dispose event handlers to clear DataSource of DGV and BindingSource of BindingNavigator thereby update their data source actual state

Customer Impact

  • A user won't catch the exception when closing a form or disposing a BindingSource

Regression?

  • Yes and no. .NET Framework 4.8 throws an exception when closing a form. v.4.7 doesn't throw the exception in this case. But anyway every version throws the exception if to Dispose a BindingSource component

Risk

  • Minimal

Screenshots

Before

  • A DataGridView doesn't update its Rows and Columns collections after its DataSource is disposed. And the DGV throws the exception when trying to redraw because the DGV is trying to draw items from a DataSource that doesn't already exist (send some index, eg. 4, to the collection that has 0 items, and catch IndexOutOfRangeException)
    vR6xyN1oe6

After

  • DataGridView and BindingNavigator subscribed on the BindingSource Disposed event and release their DataSource and BindingSource to update internal collections
    mtA3PmfUl4

Test methodology

  • Unit testing
  • Manual UI testing
  • CTI

Test environment(s)

  • .NET 6.0.100-preview.1.21101.5
  • Microsoft Windows [Version 10.0.19042.804]
Microsoft Reviewers: Open in CodeFlow

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #4551 (4c24082) into main (e805f92) will increase coverage by 0.00165%.
The diff coverage is 99.54198%.

@@                 Coverage Diff                 @@
##                main       #4551         +/-   ##
===================================================
+ Coverage   97.96079%   97.96244%   +0.00164%     
===================================================
  Files            542         543          +1     
  Lines         263533      264188        +655     
  Branches        4937        4968         +31     
===================================================
+ Hits          258159      258805        +646     
- Misses          4494        4500          +6     
- Partials         880         883          +3     
Flag Coverage Δ
Debug 97.96244% <99.54198%> (+0.00164%) ⬆️
production 100.00000% <ø> (?)
test 97.96244% <99.54198%> (+0.00164%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@vladimir-krestov vladimir-krestov added the waiting-review This item is waiting on review by one or more members of team label Feb 14, 2021
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@RussKie RussKie added 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) 📭 waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels Feb 15, 2021
@vladimir-krestov vladimir-krestov removed the 📭 waiting-author-feedback The team requires more information from the author label Feb 15, 2021
Copy link
Member

@dreddy-work dreddy-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@vladimir-krestov
Copy link
Contributor Author

Testers approved the changes ✔️

@vladimir-krestov vladimir-krestov removed the 📬 waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) label Mar 2, 2021
to DataGridView and BindingNavigator.

Fixes dotnet#4216

A DataGridView threw IndexOutOfRangeException when its DataSource
 is already disposed and the DataGridView try to redraw itself,
 because its Rows and Columns are not updated
 but DataSource is already released. The DataGridView try to draw rows
 that are not exist in DataConnection, it send some index (eg. 5)
 to items collection and catch the exception because
 this index is out of empty items collection range.

Initially, the issue repoduced when a user closes a form
with DataGridView and BindingNavigator, because the form disposes
BindingSource when closing and then disposes BindingNavigator,
that try to redraw DataGridView. It is due to BindingNavigator
send UiaReturnRawElementProvider message
to Windows and it redraws DGV sometimes (looks like a bug).
We tried to cancel DGV redwawing if a form is closing.

Then we found the second case: we cought this IndexOutOfRangeException
if to just dispose DataSource without form closing.

So the issue is DataGridView Rows and Columns are not updated when
DataSource disposing. This fix uses Dispose events to set null for
DataGridView.DataSource and BindingNavigator.BindingSource
thereby call refresh of internal collections of their items.
@vladimir-krestov vladimir-krestov force-pushed the Issue_4216-DGVThrowsException_WhenDisposingComponents branch from 0ba2b87 to 7b2c0ca Compare March 2, 2021 18:27
@RussKie RussKie merged commit 3f9c8e7 into dotnet:main Mar 2, 2021
@ghost ghost added this to the 6.0 Preview3 milestone Mar 2, 2021
RussKie pushed a commit to RussKie/winforms that referenced this pull request Mar 3, 2021
…taSource (dotnet#4551)

to DataGridView and BindingNavigator.

Fixes dotnet#4216

A DataGridView threw IndexOutOfRangeException when its DataSource
 is already disposed and the DataGridView try to redraw itself,
 because its Rows and Columns are not updated
 but DataSource is already released. The DataGridView try to draw rows
 that are not exist in DataConnection, it send some index (eg. 5)
 to items collection and catch the exception because
 this index is out of empty items collection range.

Initially, the issue repoduced when a user closes a form
with DataGridView and BindingNavigator, because the form disposes
BindingSource when closing and then disposes BindingNavigator,
that try to redraw DataGridView. It is due to BindingNavigator
send UiaReturnRawElementProvider message
to Windows and it redraws DGV sometimes (looks like a bug).
We tried to cancel DGV redwawing if a form is closing.

Then we found the second case: we cought this IndexOutOfRangeException
if to just dispose DataSource without form closing.

So the issue is DataGridView Rows and Columns are not updated when
DataSource disposing. This fix uses Dispose events to set null for
DataGridView.DataSource and BindingNavigator.BindingSource
thereby call refresh of internal collections of their items.

(cherry picked from commit 3f9c8e7)
@vladimir-krestov vladimir-krestov deleted the Issue_4216-DGVThrowsException_WhenDisposingComponents branch August 18, 2021 05:53
@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataGridView filter caused IndexOutOfRangeException
3 participants