patterncsharpMinor
Invoking worker threads
Viewed 0 times
threadsinvokingworker
Problem
I wrote this class to ensure that anything done in any of my worker threads can be displayed on the GUI via events, but I ran into non-thread-safe problems.
This class should take an action or a function with one or no params, i.e. the event delegate.
Is this code actually thread-safe, and, how could it be improved upon (simplifying the 4 bodies into two or one)?
The class is defined as:
```
private class ThreadSafeCall
{
public void Call(Action action)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
invokable.Invoke(action, null);
}
else
{
action();
}
}
catch (Exception)
{
}
}
public void Call(Action action, object param)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
invokable.Invoke(action, new object[] { param });
}
else
{
action(param);
}
}
catch (Exception)
{
}
}
public object Call(Func action)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
return invokable.Invoke(action, null);
}
else
{
return action();
}
}
catch (Exception)
{
}
return null;
}
public object Call(Func action, object param)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequire
This class should take an action or a function with one or no params, i.e. the event delegate.
Is this code actually thread-safe, and, how could it be improved upon (simplifying the 4 bodies into two or one)?
The class is defined as:
```
private class ThreadSafeCall
{
public void Call(Action action)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
invokable.Invoke(action, null);
}
else
{
action();
}
}
catch (Exception)
{
}
}
public void Call(Action action, object param)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
invokable.Invoke(action, new object[] { param });
}
else
{
action(param);
}
}
catch (Exception)
{
}
}
public object Call(Func action)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequired == true)
{
return invokable.Invoke(action, null);
}
else
{
return action();
}
}
catch (Exception)
{
}
return null;
}
public object Call(Func action, object param)
{
try
{
ISynchronizeInvoke invokable = action.Target as ISynchronizeInvoke;
if (invokable != null && invokable.InvokeRequire
Solution
Your specific usage seems to be thread-safe. But it's relying on implementation details (about lambdas), which you shouldn't do. It's also extremely easy to get it wrong.
I believe the only way it will work is if you're closing over
Because of that, I would never use code like this.
I think that you should just use
If you don't like that you need to specify the delegate type, you could write a couple of simple extension methods like:
and then use it like this (that
I believe the only way it will work is if you're closing over
this, but not any other variables and that this is a type that properly implements ISynchronizeInvoke (like a WinForms form or control). This means that it won't work if:- you're calling it from a
staticmethod
- you're only calling a
staticmethod from inside the lambda
- you're closing over some local variable or parameter
- you're calling it from another class
Because of that, I would never use code like this.
I think that you should just use
Invoke() directly, something like:Invoke(new Action(onCompleteHandle));If you don't like that you need to specify the delegate type, you could write a couple of simple extension methods like:
public static void Invoke(this ISynchronizeInvoke target, Action action)
{
target.Invoke(action, null);
}and then use it like this (that
this is required):this.Invoke(onCompleteHandle);Code Snippets
Invoke(new Action(onCompleteHandle));public static void Invoke(this ISynchronizeInvoke target, Action action)
{
target.Invoke(action, null);
}this.Invoke(onCompleteHandle);Context
StackExchange Code Review Q#28384, answer score: 5
Revisions (0)
No revisions yet.