views:

1012

answers:

2

For whatever reason, ThreadPool's QueueWorkItem doesn't return an IAsyncResult or some other handle to the work item, which would allow to wait until it's completed. There are RegisterWait... methods, but you have to pass a WaitHandle and creating them is expensive (see IAsyncResult documentation, which advises you to delay creating a WaitHandle until requested). The Task Parallel Library will fix this lack, but there is a long wait before that's available. So, are there any problems with this design:

public class Concurrent<T> {
    private ManualResetEvent _resetEvent;
    private T _result;

    public Concurrent(Func<T> f) {
        ThreadPool.QueueUserWorkItem(_ => {
                                         _result = f();
                                         if (_resetEvent != null)
                                             _resetEvent.Set();
                                     });
    }

    public WaitHandle WaitHandle {
        get {
            if (_resetEvent == null)
                _resetEvent = new ManualResetEvent(_result != null);
            return _resetEvent;
        }

    ...

EDIT: I asked a follow-up question about the concerns which arise when using async delegates instead of the ThreadPool.

+3  A: 

Well, you've got a race condition between fetching the WaitHandle and setting it. Do you really want the caller to be waiting forever if they happen to be a tiny bit late?

You should probably do some appropriate locking and keep an "I've finished" flag so that if you do create the WaitHandle after it's finished, you set it before returning it.

I'd also personally write a static factory method rather than just using a public constructor - or make it a "create and then explicitly start" pattern. Queuing the work item in the constructor feels weird to me.

Jon Skeet
Note that I pass `_result != null` as the parameter to the ManualResetEvent constructor. (This won't work for structs, but the idea is the same.) `_result != null` serves as the finishing flag here (not in the actual implementation, of course). Yes, judicious locking is needed here. (continued...)
Alexey Romanov
A static factory method goes without saying, I very rarely write a generic public constructor without a corresponding static method.
Alexey Romanov
I wouldn't make the constructor public in the first place though. You should also consider methods which return null...
Jon Skeet
As I said, `_result != null` won't be used in the actual implementation (this would also work badly if the method throws an exception).
Alexey Romanov
+2  A: 

Why aren't you using an asynchronous delegate, as demostrated here:

http://msdn.microsoft.com/en-us/library/h80ttd5f.aspx

That would make Concurrent obsolete, no?

Andreas Huber
1) Concurrent provides a more easily usable API (I think); 2) Concurrent is just one subtype of Future<T>, there are others which work differently (lazy values, promises); 3) (Most important) Because I didn't notice that Concurrent could be implemented as just a facade over asynchronous delegates.
Alexey Romanov
Ah, I thought from the start this had something to do with futures :-).
Andreas Huber