Is this an appropriate way of handling cross-thread operations?
Should I use a new property name, something like "EditValueThreadSafe" instead of overriding "EditValue"? I don't think there is an issue with the changes to the implementation of EditValue, as the base property is called regardless.
namespace MyApplication.Components
{
using System.Windows.Forms;
/// <summary>
/// Thread-safe implementation of the DevExpress.XtraEditors.ComboBoxEdit class.
/// </summary>
public class ComboBoxEditThreadSafe : DevExpress.XtraEditors.ComboBoxEdit
{
/// <summary>
/// Gets or sets the edit value.
/// </summary>
/// <value>The edit value.</value>
public override object EditValue
{
get
{
return base.EditValue;
}
set
{
if (this.InvokeRequired)
{
this.Invoke(new MethodInvoker(delegate
{
this.SetEditValue(value);
}));
}
else
{
this.SetEditValue(value);
}
}
}
/// <summary>
/// Sets the edit value.
/// </summary>
/// <param name="value">The value.</param>
private void SetEditValue(object value)
{
base.EditValue = value;
}
}
}
You can also delegate to another method that does the work, and in that method, if on the wrong thread, (BeginInvoke returns true), then call the same method back again. Doing that that eliminates the need to duplicate code.
public class ComboBoxEditThreadSafe : DevExpress.XtraEditors.ComboBoxEdit
{
public override object EditValue
{
get
{
return base.EditValue;
}
set
{
SetValue(value);
}
}
private void delegate SetValueDlg(object valeu);
private void SetValue(object value)
{
if (this.InvokeRequired)
this.BeginInvoke(
(SetValueDlg)SetValue, // calls itself, but on correct thread
new object[] { value });
else
base.editValue = value;
}
}
You can also use the Action() generic class to eliminate need to create explicit delegate class...
public class ComboBoxEditThreadSafe : DevExpress.XtraEditors.ComboBoxEdit
{
public override object EditValue
{
get { return base.EditValue; }
set { SetValue(value); }
}
private void SetValue(object value)
{
if (this.InvokeRequired)
this.BeginInvoke(
new Action<object>(SetValue), // calls itself, but on correct thread
new object[] { value });
else
base.editValue = value;
}
}
It's thread-safe, yes, though be wary of overriding a property and fundamentally changing the behaviour. Changing the implentation is fine, but this property now behaves very differently, removing the possibility of a specific exception but introducing a possible deadlock or blocking condition, which impacts on the calling code.
So yes, this is the correct use of InvokeRequired & Invoke, but I'd recommend creating a separate, purpose-specific and thread-safe property that is advertised as such.
My UI methods like yours end up looking like this:
public void setStatusLabelText(String s)
{
if (footerStatusLabel.InvokeRequired) {
StringUpdateInvokeDelegate callback = new StringUpdateInvokeDelegate(setStatusLabelText);
this.Invoke(callback, new object[] { s });
}
else {
this.footerStatusLabel.Text = s;
}
}
(this may be old for .net these days - but the point is that you can just do the operation inside this method if you are already on the right thread - makes it a little less irritating to read, but still annoying compared to Java, IMO).
I'll inject my 2 cents here. The actual calls to InvokeRequired/BeginInvoke/Invoke are not entirely thread safe. (see Avoiding the woes of Invoke/BeginInvoke in cross-thread WinForm event handling?) I would recommend finding some way of isolating the calls to these in a single place, utility api, extension method, or the like. In the article above there is complete code for a class that wraps a delegate to provide thread-safe behavior.
