CODEWITHSUNDEEP
c++multithreadingoopdesign-patterns
Josh
Josh

Reputation: 6272

Cancelling a thread running a long operation

I'm trying to work out a design predicament I have.

ClassWithLongOperation
{
    Run()
    {
        RecrusiveOperation();
    }

    RecrusiveOperation()
    {
        /* RECURSION */
    }
}

MyThread
{
    ClassWithLongOperation Op1(10);
    Op1.Run();  // Takes several minutes.

    ClassWithLongOperation Op2(20);
    Op2.Run();

    SomeOtherClassWithLongOperation Op3;
    Op3.Run();

    // Do some other stuff
}

The GUI starts MyThread, which runs for a good 5-6 minutes. I want to be able to have a big fat Cancel button on my GUI, so the user can cancel the operation.

I could create a global boolean variable bCancelled, and check if its been set in RecursiveOperation, but I want to be a good C++ & OO programmer and avoid global variables. Especially if they would have to spread across multiple files.

So how would I (following good design) safely cancel MyThread? What could I change in my setup to allow this?

I'm also using _beginthreadex to start the thread, but I could use boost if it would allow for an easier solution.

Upvotes: 6

Views: 677

Answers (5)

j_random_hacker
j_random_hacker

Reputation: 51246

Using a global variable is actually not the worst thing in the world. Having a proliferation of unnecessary global variables leads to maintenance nightmares, but it actually sounds like a quick and easy-to-understand solution here. But if you want a clean OO solution, this is certainly possible:

EDIT My original post overlooked the fact that you want to be able to run several operations in sequence, and if any of them is cancelled, none of the remaining operations are performed. This means it's more useful to keep the bool flag inside the canceller, instead of separately in each cancellable operation; and exceptions are the nicest way to handle the actual control flow. I've also tightened up a few things (added volatile for the flag itself, made names clearer, restricted unnecessary access rights).

// A thing that can cancel another thing by setting a bool to true.
class Canceller {
public:
    Canceller : cancelledFlag(false) {}

    void RegisterCancellee(Cancellee const& c) {
        c.RegisterCanceller(cancelledFlag);
    }

    void Cancel() {
        cancelledFlag = true;
    }

private:
    volatile bool cancelledFlag;
};

class CancelButton : public Canceller {
    ...
    // Call Cancel() from on-click event handler
    ...
};

class Cancellation : public std::exception {
public:
    virtual const char* what() const throw() {
        return "User cancelled operation";
    }
};

// A thing that can be cancelled by something else.
class Cancellee {
    friend class Canceller;    // Give them access to RegisterCanceller()

protected:
    Cancellee() : pCancelledFlag(0) {}

    // Does nothing if unconnected
    void CheckForCancellation() {
        if (pCancelledFlag && *pCancelledFlag) throw Cancellation();
    }

private:
    void RegisterCanceller(volatile bool& cancelledFlag) {
        pCancelledFlag = &cancelledFlag;
    }

    volatile bool* pCancelledFlag;
};

class Op1 : public Cancellee {   // (And similarly for Op2 and Op3)
    ...
    // Poll CheckForCancellation() inside main working loop
    ...
};

MyThread
{
    CancelButton cancelButton("CANCEL!");

    try {
        ClassWithLongOperation Op1(10);
        cancelButton.RegisterCancellee(Op1);
        Op1.Run();  // Takes several minutes.

        ClassWithLongOperation Op2(20);
        cancelButton.RegisterCancellee(Op2);
        Op2.Run();

        SomeOtherClassWithLongOperation Op3;
        cancelButton.RegisterCancellee(Op3);
        Op3.Run();
    } catch (Cancellation& c) {
        // Maybe write to a log file
    }

    // Do some other stuff
}

The "double bouncing" registration allows the canceller to give access to a private flag variable.

The most important thing is to not use thread termination functions, except in very specialised cases. Why? They don't run destructors. Nor do they give the target thread any chance to "clean up".

Upvotes: 3

ActiveTrayPrntrTagDataStrDrvr
ActiveTrayPrntrTagDataStrDrvr

Reputation: 315

... Or add a Stop() method to the Thread class and make the work objects be aware of the threads they're running in. You may as well throw in a Stop() method for the work objects. Depending on what's more important: Stop the thread or the work object.

Upvotes: 0

davidbuzatto
davidbuzatto

Reputation: 9424

Your flag not need to be global to your entire program, but it needs to be visible to your class code. Create the flag to be a private instance member and a public function to change it to false/true. In your recursive function, test its value to verify if the task should continue. When you want, set its value to false (through the function of course) to stop the recursive calls, i.e., when the user clicks the button you call the function in the desired instance. This way you will not break any OO principle, since you have a private flag and a public member function to safely change it.

Upvotes: 4

supertopi
supertopi

Reputation: 3488

You could implement a Stop() method for your ClassWithLongOperation and have the event handler for BigFatCancelButton to call this Stop() method for the current operation.

Upvotes: 1

Brady
Brady

Reputation: 10357

Instead of using a global variable, add a method to ClassWithLongOperation and/or MyThread, something like cancelOperation() that will set an internal boolean variable. The appropriate class methods would then need to check the variable at appropriate moments.

Upvotes: 1

Related Questions