search

MrazqRuby / googlemock

Automatically exported from code.google.com/p/googlemock
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

gmock hangs when deleting a mock object from a function mocker's dtor #79

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
This happens in environments where mutex is implemented.

The dtor of FunctionMocker grabs a global mutex, as it needs to modify the 
state of the global mock registry.  If another mock object needs to be 
delete while the FunctionMocker is being deleted, a dead lock will form.  
Here's an example:

  class MockFoo {
   public:
    void DoThis(linked_ptr<MockBar>* p);
  };
  MockFoo foo;
  linked_ptr<MockBar> bar(new MockBar);
  EXPECT_CALL(foo, DoThis(_)).WillOnce(SetArgumentPointee<0>(bar));

foo's destruction will trigger *bar's destruction, and the program hangs.

Original issue reported on code.google.com by zhanyong...@gmail.com on 29 Sep 2009 at 4:26

GoogleCodeExporter commented 9 years ago

Original comment by zhanyong...@gmail.com on 29 Oct 2009 at 4:04

GoogleCodeExporter commented 9 years ago 
FYI, just spent an hour debugging my test to figure out this was the culprit... 
would 
be nice to fix it...

Original comment by mi...@google.com on 31 Mar 2010 at 7:47

GoogleCodeExporter commented 9 years ago 
Issue 109 has been merged into this issue.

Original comment by vladlosev on 3 May 2010 at 3:35

GoogleCodeExporter commented 9 years ago 
Issue 114 has been merged into this issue.

Original comment by w...@google.com on 2 Jun 2010 at 7:59

GoogleCodeExporter commented 9 years ago

Original comment by w...@google.com on 27 Sep 2010 at 9:31

GoogleCodeExporter commented 9 years ago 
The workaround comment is very useful:

http://code.google.com/p/googlemock/issues/detail?id=114#makechanges

Original comment by zilvinas...@gmail.com on 25 Oct 2010 at 1:19

GoogleCodeExporter commented 9 years ago 
I'm upvote this if it was possible

Original comment by atomicpi...@gmail.com on 23 Jun 2011 at 1:41

GoogleCodeExporter commented 9 years ago 
Indeed, this is the biggest issue my company has in using Google Mock.

Original comment by ben.med...@gmail.com on 23 Jun 2011 at 3:46

GoogleCodeExporter commented 9 years ago 
Hi,

Deadlock occurs in FunctionMockerBase<F>::VerifyAndClearExpectationsLocked 
which is being called in destructor of mock.
Expectations vector is being cleared under protection of mutex, so if any 
expectation contains Mock as an argument / return value it deadlocks.

I've attached proposed patch for version 1.5.0 but don't know if it is correct 
in terms of threadology.

How it works:
We create temporary expectations vector, swap original one with it, unlock, 
clear temporary (thus no deadlock in destructor) and lock again.

Kudos to William Manley for help with finding source of problem and proposing 
swap trick as solution.

Original comment by shax...@gmail.com on 30 Jun 2011 at 3:46

Attachments:

GoogleCodeExporter commented 9 years ago 
Thanks for the patch - it does indeed seem to fix the problem.

But in order to accept the patch, we must ask you to sign the CLA, as outlined 
in http://code.google.com/p/googlemock/wiki/DevGuide#Contributing_Code. Let me 
know when you do that and I will merge the patch.

Original comment by vladlosev on 11 Jul 2011 at 9:53

GoogleCodeExporter commented 9 years ago 
Has this patch been merged into the project?
Thanks.

Original comment by robmccul...@gmail.com on 26 Jul 2011 at 1:03

GoogleCodeExporter commented 9 years ago 
Hello, 

What's the status of this bug ? It's really anoying ..
Thanks.

Original comment by leonard....@gmail.com on 24 Sep 2011 at 11:02

GoogleCodeExporter commented 9 years ago 
Hi,
If it is blocker for you it is still possible to apply patch on your local 
codebase.
Regards,
Z

Original comment by shax...@gmail.com on 25 Sep 2011 at 12:50

GoogleCodeExporter commented 9 years ago 
Hi,

above patch is not applicable with gmock-1.6.0 due to some code reorganization.

Thus I've slightly adapted the patch.

Could someone merge without bureaucracy or fix it in some other way in the
upstream code, please?

Regards

Original comment by hyuek...@astaro.com on 12 Oct 2011 at 10:58

Attachments:

GoogleCodeExporter commented 9 years ago 
I've implemented my own fix, with a unit test:

    http://codereview.appspot.com/5267041/

I'm a Google employee, so I believe I don't need to sign the CLA.

Original comment by jaco...@google.com on 13 Oct 2011 at 2:32

GoogleCodeExporter commented 9 years ago 
Hello,

I was contacted by maintainer shortly after submitting patch and signed CLA. It 
just waits for merging into repo.

Original comment by shax...@gmail.com on 13 Oct 2011 at 4:04

GoogleCodeExporter commented 9 years ago 
Fixed in revision 403. Thanks to Aaron Jacobs for the fix!

Original comment by vladlosev on 24 Oct 2011 at 11:42

GoogleCodeExporter commented 9 years ago 
This problem occurs in current version if you return a casted shared_ptr

Return(std::tr1::dynamic_pointer_cast<IObjectA>(SharedPtrObject))

Original comment by psp...@gmail.com on 4 Sep 2012 at 12:13

GoogleCodeExporter commented 9 years ago 
This appears to be a different, even if a related problem. If you want to, file 
a new issue for it. Don't forget o provide short but complete program that 
reproduces it and details about your compiler and OS.

Original comment by vladlosev on 5 Sep 2012 at 8:25