C++ EventHandler Class

I need to have some thread-safe event handler (like C#'s EventHandler), so I made one. Here is my event handler class and sample code (compiled with VS2013). Is this code OK to be operated in multithreaded environment?

```
#include
#include
#include
#include
#include

////////////////////////////////////////////////////////////////////////////
// Event Handler class

template
class EventHandler
{
public:
EventHandler()
{
m_flag.clear();
}

EventHandler(const EventHandler& other)
{
m_flag.clear();
m_ptr = other.m_ptr;
}

EventHandler& operator= (const EventHandler& other)
{
m_flag.clear();
m_ptr = other.m_ptr;
return *this;
}

virtual ~EventHandler()
{
while (m_flag.test_and_set(std::memory_order_acquire));
}

template
EventHandler(void (_Type::memFn)(_EventArg), _Type obj)
{
m_flag.clear();
m_ptr.reset(new std::function(std::bind(memFn, obj, std::placeholders::_1)));
}

void operator() (_EventArg e)
{
while (m_flag.test_and_set(std::memory_order_acquire));
if (m_ptr.get()) { (*m_ptr)(e); }
m_flag.clear(std::memory_order_release);
}

bool operator== (const EventHandler& other)
{
while (m_flag.test_and_set(std::memory_order_acquire));
bool equal = (m_ptr == other.m_ptr);
m_flag.clear(std::memory_order_release);
return equal;
}

private:
std::atomic_flag m_flag;
std::shared_ptr> m_ptr;
};

template
class IEvent
{
public:
IEvent()
{
m_flag.clear();
}

virtual ~IEvent()
{
while (m_flag.test_and_set(std::memory_order_acquire));
}

void operator+= (const EventHandler& handler)
{
while (m_flag.test_and_set(std::memory_order_acquire));
m_list.push_back(handler);
m_flag.clear(std::memory_order_release);
}

void operator-= (const EventHandler& handler)
{

RAII

Whenever you need to acquire a resource, hold it for a while, and then release it deterministically - think resource acquisition is initialization. RAII! When I look at your equality comparison:

bool operator== (const EventHandler& other)
{
    while (m_flag.test_and_set(std::memory_order_acquire));
    bool equal = (m_ptr == other.m_ptr);
    m_flag.clear(std::memory_order_release);
    return equal;
}

We're really doing two things. We're acquiring and releasing a lock on m_flag, and then we're doing the equality comparison. So let's make that a lot more explicit.

std::lock_guard is an RAII lock, templated on a BasicLockable - which is just a class that supports a lock() and unlock(). So instead of holding a std::atomic_flag member, we can hold a member of type:

struct FlagMutex {
    void lock() {
        while (m_flag.test_and_set(std::memory_order_acquire));
    }

    void unlock() {
        m_flag.clear(std::memory_order_release);
    }

    /*
     * note that you were not initializing your flag correctly!
     */
    std::atomic_flag m_flag = ATOMIC_INIT_FLAG;
};

And then our equality operator becomes:

bool operator== (const EventHandler& other) {
    std::lock_guard lk(m_mutex);
    return m_ptr == other.m_ptr;
}

Keep that in mind throughout. Furthermore, if you do it this way, then FlagMutex can become a template argument for EventHandler so you can actually test the assertion that a std::atomic_flag is faster than a std::mutex.

Generalize Your Class

Right now, your event handler is constructible with:

template 
EventHandler(void (_Type::*memFn)(_EventArg), _Type *obj)

But why limit to just member functions? Your class has a std::function member. That can support free functions. Just generalize it.

Additionally, prefer make_shared to just new:

using FuncType = std::function;

template ::value
          >>
EventHandler(F&& func)
{
    m_ptr = std::make_shared(std::forward(func));
}

The user can pass in a bind expression if they want, or a lambda, or any number of other things. Just lets you do more things.

Also _EventArg is a reserved name.

StackExchange Code Review Q#108366, answer score: 3