Administrator
Administrator

Reputation: 327

c# - Waiting for 1 of 2 threads to be finished

I have a place in my code, that I need to wait for a either finger to be identified on a sensor, or the user pressed a key to abort this action and return to the main menu.
I tried using something like conditional variables with Monitor and lock concepts but when I try to alert the main thread, nothing happens.

CODE:

private static object _syncFinger = new object(); // used for syncing

private static bool AttemptIdentify()
{
    // waiting for either the user cancels or a finger is inserted
    lock (_syncFinger)
    {
        Thread tEscape = new Thread(new ThreadStart(HandleIdentifyEscape));
        Thread tIdentify = new Thread(new ThreadStart(HandleIdentify));
        tEscape.IsBackground = false;
        tIdentify.IsBackground = false;
        tEscape.Start();
        tIdentify.Start();
        Monitor.Wait(_syncFinger); // -> Wait part
    }

    // Checking the change in the locked object

    if (_syncFinger is FingerData) // checking for identity found
    {
        Console.WriteLine("Identity: {0}", ((FingerData)_syncFinger).Guid.ToString());
    }
    else if(!(_syncFinger is Char)) // char - pressed a key to return
    {
        return false; // returns with no error
    }

    return true;
}

private static void HandleIdentifyEscape()
{
    do
    {
        Console.Write("Enter 'c' to cancel: ");
    } while (Console.ReadKey().Key != ConsoleKey.C);
    _syncFinger = new Char();
    LockNotify((object)_syncFinger);
}

private static void HandleIdentify()
{
    WinBioIdentity temp = null;
    do
    {
        Console.WriteLine("Enter your finger.");
        try // trying to indentify
        {
            temp = Fingerprint.Identify(); // returns FingerData type
        }
        catch (Exception ex)
        {
            Console.WriteLine("ERROR: " + ex.Message);
        }
        // if couldn't identify, temp would stay null
        if(temp == null)
        {
            Console.Write("Invalid, ");
        }
    } while (temp == null);

    _syncFinger = temp;
    LockNotify(_syncFinger);
}

private static void LockNotify(object syncObject)
{
    lock(syncObject)
    {
        Monitor.Pulse(syncObject); 
    }
}

Upvotes: 3

Views: 240

Answers (1)

Peter Duniho
Peter Duniho

Reputation: 70691

when i try to alert the main thread, nothing happens.

That's because the main thread is waiting on the monitor for the object created here:

private static object _syncFinger = new object(); // used for syncing

But each of your threads replaces that object value, and then signals the monitor for the new object. The main thread has no knowledge of the new object, and so of course signaling the monitor for that new object will have no effect on the main thread.

First, any time you create an object for the purpose of using with lock, make it readonly:

private static readonly object _syncFinger = new object(); // used for syncing

It's always the right thing to do , and that will prevent you from ever making the mistake of changing the monitored object while a thread is waiting on it.

Next, create a separate field to hold the WinBioIdentity value, e.g.:

private static WinBioIdentity _syncIdentity;

And use that to relay the result back to the main thread:

private static bool AttemptIdentify()
{
    // waiting for either the user cancels or a finger is inserted
    lock (_syncFinger)
    {
        _syncIdentity = null;
        Thread tEscape = new Thread(new ThreadStart(HandleIdentifyEscape));
        Thread tIdentify = new Thread(new ThreadStart(HandleIdentify));
        tEscape.IsBackground = false;
        tIdentify.IsBackground = false;
        tEscape.Start();
        tIdentify.Start();
        Monitor.Wait(_syncFinger); // -> Wait part
    }

    // Checking the change in the locked object

    if (_syncIdentity != null) // checking for identity found
    {
        Console.WriteLine("Identity: {0}", ((FingerData)_syncIdentity).Guid.ToString());
        return true;
    }

    return false; // returns with no error
}

private static void HandleIdentifyEscape()
{
    do
    {
        Console.Write("Enter 'c' to cancel: ");
    } while (Console.ReadKey().Key != ConsoleKey.C);
    LockNotify((object)_syncFinger);
}

private static void HandleIdentify()
{
    WinBioIdentity temp = null;
    do
    {
        Console.WriteLine("Enter your finger.");
        try // trying to indentify
        {
            temp = Fingerprint.Identify(); // returns FingerData type
        }
        catch (Exception ex)
        {
            Console.WriteLine("ERROR: " + ex.Message);
        }
        // if couldn't identify, temp would stay null
        if(temp == null)
        {
            Console.Write("Invalid, ");
        }
    } while (temp == null);

    __syncIdentity = temp;
    LockNotify(_syncFinger);
}

All that said, you should prefer to use the modern async/await idiom for this:

private static bool AttemptIdentify()
{
    Task<WinBioIdentity> fingerTask = Task.Run(HandleIdentify);
    Task cancelTask = Task.Run(HandleIdentifyEscape);

    if (Task.WaitAny(fingerTask, cancelTask) == 0)
    {
        Console.WriteLine("Identity: {0}", fingerTask.Result.Guid);
        return true;
    }

    return false;
}

private static void HandleIdentifyEscape()
{
    do
    {
        Console.Write("Enter 'c' to cancel: ");
    } while (Console.ReadKey().Key != ConsoleKey.C);
}

private static WinBioIdentity HandleIdentify()
{
    WinBioIdentity temp = null;
    do
    {
        Console.WriteLine("Enter your finger.");
        try // trying to indentify
        {
            temp = Fingerprint.Identify(); // returns FingerData type
        }
        catch (Exception ex)
        {
            Console.WriteLine("ERROR: " + ex.Message);
        }
        // if couldn't identify, temp would stay null
        if(temp == null)
        {
            Console.Write("Invalid, ");
        }
    } while (temp == null);

    return temp;
}

The above is a bare-minimum example. It would be better to make the AttemptIdentify() method async itself, and then use await Task.WhenAny() instead of Task.WaitAny(). It would also be better to include some mechanism to interrupt the tasks, i.e. once one has completed, you should want to interrupt the other so it's not lying around continuing to attempt its work.

But those kinds of issues are not unique to the async/await version, and don't need to be solved to improve on the code you have now.

Upvotes: 5

Related Questions