Skip to content

Commit

Permalink
Fixing IndexOutOfRangeException throwing by DGV when disposing its Da…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
vladimir-krestov authored and RussKie committed Mar 3, 2021
1 parent 937b05a commit 5c06649
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,14 @@ private void OnBindingSourceStateChanged(object sender, EventArgs e)
RefreshItemsInternal();
}

/// <summary>
/// Refresh tool strip items when the BindingSource is disposed.
/// </summary>
private void OnBindingSourceDisposed(object sender, EventArgs e)
{
BindingSource = null;
}

/// <summary>
/// Refresh tool strip items when something changes in the BindingSource's list.
/// </summary>
Expand Down Expand Up @@ -895,6 +903,7 @@ private void WireUpBindingSource(ref BindingSource oldBindingSource, BindingSour
oldBindingSource.DataSourceChanged -= new EventHandler(OnBindingSourceStateChanged);
oldBindingSource.DataMemberChanged -= new EventHandler(OnBindingSourceStateChanged);
oldBindingSource.ListChanged -= new ListChangedEventHandler(OnBindingSourceListChanged);
oldBindingSource.Disposed -= new EventHandler(OnBindingSourceDisposed);
}

if (newBindingSource != null)
Expand All @@ -905,6 +914,7 @@ private void WireUpBindingSource(ref BindingSource oldBindingSource, BindingSour
newBindingSource.DataSourceChanged += new EventHandler(OnBindingSourceStateChanged);
newBindingSource.DataMemberChanged += new EventHandler(OnBindingSourceStateChanged);
newBindingSource.ListChanged += new ListChangedEventHandler(OnBindingSourceListChanged);
newBindingSource.Disposed += new EventHandler(OnBindingSourceDisposed);
}

oldBindingSource = newBindingSource;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14903,6 +14903,14 @@ protected virtual void OnDataSourceChanged(EventArgs e)
}
}

/// <summary>
/// Refresh items when the DataSource is disposed.
/// </summary>
private void OnDataSourceDisposed(object sender, EventArgs e)
{
DataSource = null;
}

protected virtual void OnDefaultCellStyleChanged(EventArgs e)
{
if (e is DataGridViewCellStyleChangedEventArgs dgvcsce && !dgvcsce.ChangeAffectsPreferredSize)
Expand Down
10 changes: 10 additions & 0 deletions src/System.Windows.Forms/src/System/Windows/Forms/DataGridView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,16 @@ public object DataSource
{
if (value != DataSource)
{
if (DataSource is Component oldDataSource)
{
oldDataSource.Disposed -= OnDataSourceDisposed;
}

if (value is Component newDataSource)
{
newDataSource.Disposed += OnDataSourceDisposed;
}

CurrentCell = null;
if (DataConnection is null)
{
Expand Down
82 changes: 82 additions & 0 deletions src/System.Windows.Forms/tests/UnitTests/BindingNavigatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Diagnostics;
using Moq;
using Xunit;
Expand Down Expand Up @@ -129,5 +130,86 @@ public void BindingNavigator_ConstructorBool()
Assert.Equal(bn.AddNewItem, bn.Items[index++]);
Assert.Equal(bn.DeleteItem, bn.Items[index++]);
}

[WinFormsFact]
public void BindingNavigator_UpdatesItsItems_AfterDataSourceDisposing()
{
using BindingNavigator control = new BindingNavigator(true);
int rowsCount = 5;
BindingSource bindingSource = GetTestBindingSource(rowsCount);
control.BindingSource = bindingSource;

Assert.Equal("1", control.PositionItem.Text);
Assert.Equal($"of {rowsCount}", control.CountItem.Text);

bindingSource.Dispose();

// The BindingNavigator updates its PositionItem and CountItem values
// after its BindingSource is disposed
Assert.Equal("0", control.PositionItem.Text);
Assert.Equal("of 0", control.CountItem.Text);
}

[WinFormsFact]
public void BindingNavigator_BindingSource_IsNull_AfterDisposing()
{
using BindingNavigator control = new BindingNavigator();
BindingSource bindingSource = GetTestBindingSource(5);
control.BindingSource = bindingSource;

Assert.Equal(bindingSource, control.BindingSource);

bindingSource.Dispose();

Assert.Null(control.BindingSource);
}

[WinFormsFact]
public void BindingNavigator_BindingSource_IsActual_AfterOldOneIsDisposed()
{
using BindingNavigator control = new BindingNavigator(true);
int rowsCount1 = 3;
BindingSource bindingSource1 = GetTestBindingSource(rowsCount1);
int rowsCount2 = 5;
BindingSource bindingSource2 = GetTestBindingSource(rowsCount2);
control.BindingSource = bindingSource1;

Assert.Equal(bindingSource1, control.BindingSource);
Assert.Equal("1", control.PositionItem.Text);
Assert.Equal($"of {rowsCount1}", control.CountItem.Text);

control.BindingSource = bindingSource2;

Assert.Equal(bindingSource2, control.BindingSource);
Assert.Equal("1", control.PositionItem.Text);
Assert.Equal($"of {rowsCount2}", control.CountItem.Text);

bindingSource1.Dispose();

// bindingSource2 is actual for the BindingNavigator
// so it will contain correct PositionItem and CountItem values
// even after bindingSource1 is disposed.
// This test checks that Disposed events unsubscribed correctly
Assert.Equal(bindingSource2, control.BindingSource);
Assert.Equal("1", control.PositionItem.Text);
Assert.Equal($"of {rowsCount2}", control.CountItem.Text);
}

private BindingSource GetTestBindingSource(int rowsCount)
{
DataTable dt = new DataTable();
dt.Columns.Add("Name");
dt.Columns.Add("Age");

for (int i = 0; i < rowsCount; i++)
{
DataRow dr = dt.NewRow();
dr[0] = $"User{i}";
dr[1] = i * 3;
dt.Rows.Add(dr);
}

return new() { DataSource = dt };
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Numerics;
using static System.Windows.Forms.Metafiles.DataHelpers;
using static Interop;
using System.Data;

namespace System.Windows.Forms.Tests
{
Expand Down Expand Up @@ -2801,6 +2802,92 @@ public void DataGridView_OnRowHeadersWidthSizeModeChanged_NullE_ThrowsNullRefere
Assert.Throws<NullReferenceException>(() => control.OnRowHeadersWidthSizeModeChanged(null));
}

[WinFormsFact]
public void DataGridView_UpdatesItsItems_AfterDataSourceDisposing()
{
using DataGridView control = new DataGridView();
int rowsCount = 5;
BindingSource bindingSource = GetTestBindingSource(rowsCount);
BindingContext context = new BindingContext();
context.Add(bindingSource, bindingSource.CurrencyManager);
control.BindingContext = context;
control.DataSource = bindingSource;

// The TestBindingSource table contains 2 columns
Assert.Equal(2, control.Columns.Count);
// The TestBindingSource table contains some rows + 1 new DGV row (because AllowUserToAddRows is true)
Assert.Equal(rowsCount + 1, control.Rows.Count);

bindingSource.Dispose();

// The DataGridView updates its Rows and Columns collections after its DataSource is disposed
Assert.Empty(control.Columns);
Assert.Empty(control.Rows);
}

[WinFormsFact]
public void DataGridView_DataSource_IsNull_AfterDisposing()
{
using DataGridView control = new DataGridView();
BindingSource bindingSource = GetTestBindingSource(5);
control.DataSource = bindingSource;

Assert.Equal(bindingSource, control.DataSource);

bindingSource.Dispose();

Assert.Null(control.DataSource);
}

[WinFormsFact]
public void DataGridView_DataSource_IsActual_AfterOldOneIsDisposed()
{
using DataGridView control = new DataGridView();
int rowsCount1 = 3;
BindingSource bindingSource1 = GetTestBindingSource(rowsCount1);
int rowsCount2 = 5;
BindingSource bindingSource2 = GetTestBindingSource(rowsCount2);
BindingContext context = new BindingContext();
context.Add(bindingSource1, bindingSource1.CurrencyManager);
control.BindingContext = context;
control.DataSource = bindingSource1;

Assert.Equal(bindingSource1, control.DataSource);
Assert.Equal(2, control.Columns.Count);
Assert.Equal(rowsCount1 + 1, control.Rows.Count); // + 1 is the new DGV row

control.DataSource = bindingSource2;

Assert.Equal(bindingSource2, control.DataSource);
Assert.Equal(2, control.Columns.Count);
Assert.Equal(rowsCount2 + 1, control.Rows.Count); // + 1 is the new DGV row

bindingSource1.Dispose();

// bindingSource2 is actual for the DataGridView so it will contain correct Rows and Columns counts
// even after bindingSource1 is disposed. This test checks that Disposed events unsubscribed correctly
Assert.Equal(bindingSource2, control.DataSource);
Assert.Equal(2, control.Columns.Count);
Assert.Equal(rowsCount2 + 1, control.Rows.Count); // + 1 is the new DGV row
}

private BindingSource GetTestBindingSource(int rowsCount)
{
DataTable dt = new DataTable();
dt.Columns.Add("Name");
dt.Columns.Add("Age");

for (int i = 0; i < rowsCount; i++)
{
DataRow dr = dt.NewRow();
dr[0] = $"User{i}";
dr[1] = i * 3;
dt.Rows.Add(dr);
}

return new() { DataSource = dt };
}

private class SubDataGridViewCell : DataGridViewCell
{
}
Expand Down

0 comments on commit 5c06649

Please sign in to comment.