.NET ThreadPool QueueUserWorkItem Synchronization

I am employing ThreadPool.QueueUserWorkItem to play some sound files and not hanging up the GUI while doing so.

It is working but has an undesirable side effect.

While the QueueUserWorkItem CallBack Proc is being executed there is nothing to stop it from starting a new thread. This causes the samples in the threads to overlap.

How can I make it so that it 开发者_Go百科waits for the already running thread to finish running and only then run the next request?

EDIT: private object sync = new Object(); lock (sync) { .......do sound here }

this works. plays in the sounds in order.

but some samples are getting played more than once when i keep sending sound requests before the one being played ends. will investigate.

EDIT: is the above a result of Lock Convoy @Aaronaught mentioned?

This is a classic thread synchronization issue, where you have multiple clients that all want to use the same resource and need to control how they access it. In this particular case, the sound system is willing to play more than one sound at the same time (and this is often desirable), but since you don't want that behavior, you can use standard locking to gate access to the sound system:

public static class SequentialSoundPlayer
{
    private static Object _soundLock = new object();

    public static void PlaySound(Sound sound)
    {
        ThreadPool.QueueUserWorkItem(AsyncPlaySound, sound);
    }

    private static void AsyncPlaySound(Object state)
    {
        lock (_soundLock)
        {
            Sound sound = (Sound) state;
            //Execute your sound playing here...
        }
    }
}

where Sound is whatever object you're using to represent a sound to be played. This mechanism is 'first come, first served' when multiple sounds vie for play time.

As mentioned in another response, be careful of excessive 'pile-up' of sounds, as you'll start to tie up the ThreadPool.

You could use a single thread with a queue to play all the sounds.

When you want to play a sound, insert a request into the queue and signal to the playing thread that there is a new sound file to be played. The sound playing thread sees the new request and plays it. Once the sound completes, it checks to see if there are any more sounds in the queue and if so plays the next, otherwise it waits for the next request.

One possible problem with this method is that if you have too many sounds that need to be played you can get an evergrowing backlog so that sounds may come several seconds or possibly even minutes late. To avoid this you might want to put a limit on the queue size and drop some sounds if you have too many.

A very simple producer/consumer queue would be ideal here - since you only have 1 producer and 1 consumer you can do it with minimal locking.

Don't use a critical section (lock statement) around the actual Play method/operation as some people are suggesting, you can very easily end up with a lock convoy. You do need to lock, but you should only be doing it for very short periods of time, not while a sound is actually playing, which is an eternity in computer time.

Something like this:

public class SoundPlayer : IDisposable
{
    private int maxSize;
    private Queue<Sound> sounds = new Queue<Sound>(maxSize);
    private object sync = new Object();
    private Thread playThread;
    private bool isTerminated;

    public SoundPlayer(int maxSize)
    {
        if (maxSize < 1)
            throw new ArgumentOutOfRangeException("maxSize", maxSize,
                "Value must be > 1.");
        this.maxSize = maxSize;
        this.sounds = new Queue<Sound>();
        this.playThread = new Thread(new ThreadStart(ThreadPlay));
        this.playThread.Start();
    }

    public void Dispose()
    {
        isTerminated = true;
        lock (sync)
        {
            Monitor.PulseAll(sync);
        }
        playThread.Join();
    }

    public void Play(Sound sound)
    {
        lock (sync)
        {
            if (sounds.Count == maxSize)
            {
                return;   // Or throw exception, or block
            }
            sounds.Enqueue(sound);
            Monitor.PulseAll(sync);
        }
    }

    private void PlayInternal(Sound sound)
    {
        // Actually play the sound here
    }

    private void ThreadPlay()
    {
        while (true)
        {
            lock (sync)
            {
                while (!isTerminated && (sounds.Count == 0))
                    Monitor.Wait(sync);
                if (isTerminated)
                {
                    return;
                }
                Sound sound = sounds.Dequeue();
                Play(sound);
            }
        }
    }
}

This will allow you to throttle the number of sounds being played by setting maxSize to some reasonable limit, like 5, after which point it will simply discard new requests. The reason I use a Thread instead of ThreadPool is simply to maintain a reference to the managed thread and be able to provide proper cleanup.

This only uses one thread, and one lock, so you'll never have a lock convoy, and will never have sounds playing at the same time.

If you're having any trouble understanding this, or need more detail, have a look at Threading in C# and head over to the "Producer/Consumer Queue" section.

The simplest code you could write would be as follows:

private object playSoundSync = new object();
public void PlaySound(Sound someSound)
{
    ThreadPool.QueueUserWorkItem(new WaitCallback(delegate
    {
      lock (this.playSoundSync)
      {
        PlaySound(someSound);
      }
    }));
}

Allthough very simple it pontentially could yield problems:

1. If you play a lot of (longer) sounds simultaneously there will be a lot of locks and a lot of threadpool threads get used up.
2. The order you enqueued the sounds is not necesesarily the order they will be played back.

in practise these problems should only be relevant if you play a lot of sounds frequently or if the sounds are very long.

Another option, if you can make the (major) simplifying assumption that any attempts to play a second sound while the first is still playing will just be ignored, is to use a single event:

private AutoResetEvent playEvent = new AutoResetEvent(true);

public void Play(Sound sound)
{
    ThreadPool.QueueUserWorkItem(s =>
    {
        if (playEvent.WaitOne(0))
        {
            // Play the sound here
            playEvent.Set();
        }
    });
}

This one's dead easy, with the obvious disadvantage that it will simply discard "extra" sounds instead of queuing them. But in this case, it may be exactly what you want, and we get to use the thread pool because this function will return instantly if a sound is already playing. It's basically "lock-free."

As per your edit, create your thread like this:

MySounds sounds = new MySounds(...);
Thread th = new Thread(this.threadMethod, sounds);
th.Start();

And this will be your thread entry point.

private void threadMethod (object obj)
{
    MySounds sounds = obj as MySounds;
    if (sounds == null) { /* do something */ }

    /* play your sounds */
}

The use of ThreadPool is not the error. The error is queueing every sound as work item. Naturally the thread pool will start more threads. This is what it is supposed to do.

Build your own queue. I have one (AsyncActionQueue). It queues items and when it has an item it will start a ThreadPool WorkItem - not one per item, ONE (unless one is already queued and not finished). The callback basically unqeueues items and processes them.

This allows me to have X queues share Y threads (i.e. not waste threads) and still get very nice async operations. I use that for a comples UI trading application - X windows (6, 8) communicating with a central service cluster (i.e. a number of services) and they all use async queues to move items back and forth (well, mostly forth towards the UI).

One thing you NEED to be aware of - and that has been said already - is that if you overload your queue, it will fall back. What to do then depends on your. I have a ping/pong message that gets queued regularly to a service (from the window) and if not returned in time, the window goes grey marking "I am stale" until it catches up.

Microsoft's new TPL Dataflow Library could be a good solution for this sort of thing. Check out the video here - the first code example demonstrated fits your requirements pretty much exactly.

http://channel9.msdn.com/posts/TPL-Dataflow-Tour