#47 Creating/Deleting mocked objects not thread-safe

[Ronny Spiegel]

I just stumbled across the fact that Turtle Mock is not thread-safe.

You will run into issues if you create or delete mocked objects simultaneously. The reason is that every class created using MOCK_CLASS indirectly derives from a class that is called “verifiable” and all instances of these “verifiable” class will be stored in a vector inside of Turtle-Mock upon creation and removed from this vector upon deletion. This access to the vector is not synchronized and will cause crashes if you don’t care.

My code was looking like this:

static MockedObjectPtr createMockedObjectPtr(uuid const & object_id)
{
    MockedObjectPtr object = make_intrusive<MockedObject>();
    return object;
}

and later:

MOCK_EXPECT((*dist_access.at(i)).getObject_mock)
    .with(object_id, mock::any)
    .calls([](uuid const &, LockedObject& output)
    {
           output = createMockedObjectPtr(object_id);
           return true;
    });

This expectation is being called from multiple tests concurrently. This led to crashes from time to time.

And now it looks like this (where the MOCK-Object is created outside of the expectation as part of the test setup):

MockedObjectPtr obj = createMockedObjectPtr(object_id);
MOCK_EXPECT((*dist_access.at(i)).getObject_mock)
    .with(object_id, mock::any)
    .calls([obj](uuid const &, LockedObject& output)
    {
           output = obj;
           return true;
    });

I propose to add synchronization mechanisms to Turtle to avoid such crashes. Synchronization mechanisms might get injected via extension points to avoid creating a dependency to a threading library. If Turtle Mock relies on C++11 anyway std::mutex might be used.

What's your opinion on that?

[Mathieu Champlon]

Hi Ronny,

Thread-safety has been on my to-do list for quite some time now.
I even took a stab at it a couple of weeks ago !
I was thinking making it all optional using a define.

Turtle has support for some C++11 features but it should fall back to C++03 when needed.
However this idiom is already used in several places so this shouldn't be a problem.

Thanks for your debugging, I'll keep you posted !

Cheers,
MAT.

[Mathieu Champlon]

Thread safety has been introduced in http://sourceforge.net/p/turtle/code/730/
In order to activate the feature the symbol MOCK_THREAD_SAFE must be defined before including the library.

Does this meet your expectations?
I'll make a new release shortly but in the meanwhile any feedback will be greatly appreciated!

Thanks,
MAT.

[Andreas Weis]

Thank you very much for the fix!

One thing I noticed: The mock::detail::functor default constructor uses a function-scope static mutex for synchronization. This is safe on all compilers implementing the C++11 feature N2660 ('magic statics').

Unfortunately, this feature is not available on any Visual C++ version (except the very latest VC12 November 2013 CTP, which does not come with a 'Go Live' license). It would be great if you could change the implementation to make it thread-safe on those compilers as well.

[Mathieu Champlon]

That's a good point!
Do you have any suggestion?

MAT.

[Mathieu Champlon]

Right, I believe it to be fixed in https://sourceforge.net/p/turtle/code/733/
Does that seem OK to you?

Thanks for pointing that out!

MAT.

[Andreas Weis]

That should do the trick.

What is a bit annoying is that you will now get a new global instance of the functor_mutex reference in every .cpp file that pulls that header in (via the expanded BOOST_TEST_SINGLETON_INST macro), but I don't see a way to avoid this in a header-only library.

[Mathieu Champlon]

Thanks again for your help !

MAT.

[Ronny Spiegel]

Thank you very much for the fix!

Am 29.05.2014 um 14:42 schrieb Mathieu Champlon mat007@users.sf.net:

status: pending --> closed
Comment:
Thanks again for your help !

MAT.

[tickets:#47] Creating/Deleting mocked objects not thread-safe

Status: closed
Labels: thread safety
Created: Tue Feb 11, 2014 12:54 PM UTC by Ronny Spiegel
Last Updated: Wed May 21, 2014 12:28 PM UTC
Owner: Mathieu Champlon

I just stumbled across the fact that Turtle Mock is not thread-safe.

You will run into issues if you create or delete mocked objects simultaneously. The reason is that every class created using MOCK_CLASS indirectly derives from a class that is called “verifiable” and all instances of these “verifiable” class will be stored in a vector inside of Turtle-Mock upon creation and removed from this vector upon deletion. This access to the vector is not synchronized and will cause crashes if you don’t care.

My code was looking like this:

static MockedObjectPtr createMockedObjectPtr(uuid const & object_id)
{
MockedObjectPtr object = make_intrusive<MockedObject>();
return object;
}
and later:

MOCK_EXPECT((*dist_access.at(i)).getObject_mock)
.with(object_id, mock::any)
.calls(
{
output = createMockedObjectPtr(object_id);
return true;
});
This expectation is being called from multiple tests concurrently. This led to crashes from time to time.

And now it looks like this (where the MOCK-Object is created outside of the expectation as part of the test setup):

MockedObjectPtr obj = createMockedObjectPtr(object_id);
MOCK_EXPECT((*dist_access.at(i)).getObject_mock)
.with(object_id, mock::any)
.calls(obj
{
output = obj;
return true;
});
I propose to add synchronization mechanisms to Turtle to avoid such crashes. Synchronization mechanisms might get injected via extension points to avoid creating a dependency to a threading library. If Turtle Mock relies on C++11 anyway std::mutex might be used.

What's your opinion on that?

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/turtle/tickets/47/

To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/