4
\$\begingroup\$

In the main class, loops generate numbers (0~100), and when its generated number is > 20, its value is passed to the thread where it simulates some work with this number. Meanwhile, while this number is being processed, other generated numbers > 20 must be skipped.

Is this code OK? I am not sure if I'm doing something bad or not. In fact it seems to be working fine, but I don't know if it can be written by that way or even "better.".

class Program
{
    delegate void SetNumberDelegate(int number);
    delegate bool IsBusyDelegate();

    static void Main(string[] args)
    {
        Random rnd = new Random();
        ProcessingClass processClass = new ProcessingClass();
        SetNumberDelegate setNum = new SetNumberDelegate(processClass.setNumber);
        IsBusyDelegate isBusy = new IsBusyDelegate(processClass.isBusy);
        Thread processThread = new Thread(new ThreadStart(processClass.processNumbers));
        processThread.Start();
        int num;
        int count = 0;
        while (count++ < 100)
        {
            num = rnd.Next(0, 100);
            Console.WriteLine("Generated number {0}", num);
            if (num > 20)
            {
                if (!isBusy())
                {
                    setNum(num);
                }
                else
                {
                    Console.WriteLine("Thread BUSY! Skipping number:{0}", num);
                }
            }
            Thread.Sleep(1);
        }
        processThread.Abort();
        processThread.Join();
        Console.ReadKey();
    }
}
class ProcessingClass
{
    private volatile bool busy;
    private volatile int number;

    public ProcessingClass()
    {
        busy = false;
        number = -1;
    }

    public bool isBusy()
    {
        return busy;
    }

    public void setNumber(int num)
    {
        number = num;
    }

    public void processNumbers()
    {
        while (true)
        {
            if (number > -1 && !busy)
            {
                busy = true;
                Console.WriteLine("Processing number:{0}", number);
                // simulate some work with number e.g. computing and storing to db
                Thread.Sleep(500); 
                Console.WriteLine("Done");
                number = -1;
                busy = false;
            }
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

There is no synchronisation here and so all access to the thread's state is subject to data races. It is comprehensively not threadsafe.

From what I can tell all you need is to implement a producer/consumer pattern. Do this with a blocking queue. Please don't use busy loops. They burn CPU cycles and are horribly non-performant.

\$\endgroup\$
  • 2
    \$\begingroup\$ any short code sample, please? \$\endgroup\$ – user832347 Sep 17 '11 at 18:34
3
\$\begingroup\$

Why not use BackgroundWorker?

static void Main(string[] args)
{
    Random rnd = new Random();
    int count = 0, num;

    BackgroundWorker bw = new BackgroundWorker();
    bw.WorkerReportsProgress = true;
    bw.WorkerSupportsCancellation = true;
    bw.DoWork += new DoWorkEventHandler(bw_DoWork);

    while(count++ < 100)
    {
        num = rnd.Next(0, 100)
        Console.WriteLine("Generated number {0}", num);
        if(num > 20)
        {
            if(!bw.IsBusy)
                bw.RunWorkerAsync(); //calls bw_DoWork function
            else
                Console.WriteLine("Thread BUSY! Skipping number:{0}", num);
        }   
    }
    void bw_DoWork(object sender, DoWorkEventArgs e)
    {
        //do something with number in background
    }
}

If you wish to abort, call bw.CancelWorkerAsync().

\$\endgroup\$
2
\$\begingroup\$

I think this should do what you expected to do. Enqueue all Numbers greater than 20 if no number is in the queue of the thread.
Please leave a comment if you wanted to do something else, i would be glad giving you the right examples.

class Program
{
    delegate void SetNumberDelegate (int number);

    delegate bool IsBusyDelegate ();

    static void Main (string[] args) {
        Random rnd = new Random ();
        ProcessingClass processClass = new ProcessingClass ();

        ManualResetEvent resetEvent = new ManualResetEvent (false);
        ThreadPool.QueueUserWorkItem (processClass.ProcessNumbers, resetEvent);

        int num;
        int count = 0;
        while (count++ < 100) {
            num = rnd.Next (0, 100);
            Console.WriteLine ("Generated number {0}", num);

            if (num > 20) {
                if (!processClass.IsBusy) {
                    processClass.Enqueue (num);
                } else {
                    Console.WriteLine ("Thread BUSY! Skipping number:{0}", num);
                }
            }
            System.Threading.Thread.Sleep (1);
        }
        resetEvent.WaitOne ();
        Console.ReadKey ();
    }
}

class ProcessingClass
{
    readonly Queue  numberQueue = Queue.Synchronized (new Queue ());
    bool aborting = false;
    object abortingLock = new object ();

    public bool Aborting {
        get {
            lock (abortingLock) {
                return aborting;
            }
        }
        set {
            lock (abortingLock) {
                aborting = value;
            }
        }
    }

    public bool IsBusy {
        get {
            return numberQueue.Count > 0;
        }
    }

    public void Abort () {
        Aborting = true;
    }

    public void Enqueue (int num) {
        lock (numberQueue) {
            numberQueue.Enqueue (num);
            Monitor.PulseAll (numberQueue);
        }

    }

    public int Dequeue () {
        lock (numberQueue) {
            if (numberQueue.Count == 0)
                Monitor.Wait (numberQueue); 
            return (int)numberQueue.Dequeue ();
        }

    }

    public void ProcessNumbers (object threadContext) {
        //please create an own class for this!
        ManualResetEvent resetEvent = (ManualResetEvent)threadContext;
        //
        StartProcessingNumbers ();
        resetEvent.Set ();

    }

    private void StartProcessingNumbers () {
        while (!aborting) {
            int num = Dequeue ();
            Console.WriteLine ("Processing number:{0}", num);
            // simulate some work with number e.g. computing and storing to db
            System.Threading.Thread.Sleep (200); 
            Console.WriteLine ("Done");             
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

This is NOT thread-safe at all... for this sort of thing (Producer/Consumer) use BlockingCollection - see MSDN reference including code sample at http://msdn.microsoft.com/en-us/library/dd997371.aspx

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.