Issue with mock with shared_ptr used in self-invocation

[ericlemes]

I've created a test to capture this issue, but didn't find out how to fix yet.

    class ISomeInterface3 {
    public:
        virtual ~ISomeInterface3() { }

        virtual void methodWithSomeSharedPointer(std::shared_ptr<ISomeInterface3> listener) = 0;
    };

    void production_shared_ptr_mock_used_in_invocation() {      
        Mock<ISomeInterface3> mockDevAssertListener;
        std::shared_ptr<ISomeInterface3> devAssertListener = std::shared_ptr<ISomeInterface3>(&mockDevAssertListener.get());

        fakeit::Fake(Dtor(mockDevAssertListener));                      
        fakeit::Fake(Method(mockDevAssertListener, methodWithSomeSharedPointer));

        devAssertListener->methodWithSomeSharedPointer(devAssertListener);
        devAssertListener = nullptr;
    }

I think this issue happens because some of the invocations store the shared_ptr and because of the order things are destructed it calls the destructor after the internal mock object is killed.

[eranpeer]

That was a great help. I think I resolved this issue. I've just pushed the fix. Thanks,

On Mon, Aug 13, 2018 at 4:32 AM Eric Lemes notifications@github.com wrote:

I've created a test to capture this issue, but didn't find out how to fix yet.

class ISomeInterface3 { public: virtual ~ISomeInterface3() { }

  virtual void methodWithSomeSharedPointer(std::shared_ptr<ISomeInterface3> listener) = 0;

};

void production_shared_ptr_mock_used_in_invocation() {
Mock mockDevAssertListener; std::shared_ptr devAssertListener = std::shared_ptr(&mockDevAssertListener.get());

  fakeit::Fake(Dtor(mockDevAssertListener));                      
  fakeit::Fake(Method(mockDevAssertListener, methodWithSomeSharedPointer));

  devAssertListener->methodWithSomeSharedPointer(devAssertListener);
  devAssertListener = nullptr;

}

I think this issue happens because some of the invocations store the shared_ptr and because of the order things are destructed it calls the destructor after the internal mock object is killed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8gpZ4Dq6ysldXM4cX0cI0TAwSwgcnks5uQWO1gaJpZM4V6Vv5 .

-- Eran 1-424-2504000

[ericlemes]

Good to know that helped. I was struggling to fix it, since I don't understand the code very well. I'll get there eventually.

If the test pass, it is definitely fixed.

[eranpeer]

I’ve just noticed that the build is broken on master. It is my mistake. I assumed that it will compile on Linux-gcc. I was wrong. Working on it now.

Sent from Mail for Windows 10

From: Eric Lemes Sent: Wednesday, August 15, 2018 12:06 AM To: eranpeer/FakeIt Cc: eranpeer; Comment Subject: Re: [eranpeer/FakeIt] Issue with mock with shared_ptr used inself-invocation (#153)

Good to know that helped. I was struggling to fix it, since I don't understand the code very well. I'll get there eventually. If the test pass, it is definitely fixed. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[eranpeer]

I fixed it!. I added a new api: mock.getShared() that returns a shared_ptr of the mock instance. It is like mock.get() but it returns a shared_ptr. Since fakeit is the owner of the mock instance, any attempt to wrap it with a shared_ptr will result in the shared_ptr deleting the instance when the reference count is 0. in order to avoid that simply obtain the shared_ptr from the mock.getShared() new api:

class ISomeInterface3 { public: virtual ~ISomeInterface3() {}

virtual void methodWithSomeSharedPointer(std::shared_ptr listener) = 0; };

void production_shared_ptr_mock_used_in_invocation() { std::shared_ptr pMockInstance; Mock mock; fakeit::Fake(Dtor(mock)); fakeit::Fake(Method(mock, methodWithSomeSharedPointer));

pMockInstance = mock.getShared(); pMockInstance->methodWithSomeSharedPointer(pMockInstance);

pMockInstance = nullptr; }

I think I can solve it in another way without introducing a new API. Let me know what you think about this approach, maybe a better name for getShared()?

On Wed, Aug 15, 2018 at 12:14 AM Eran Pe'er eranpe@gmail.com wrote:

I’ve just noticed that the build is broken on master. It is my mistake. I assumed that it will compile on Linux-gcc. I was wrong. Working on it now.

Sent from Mail https://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10

From: Eric Lemes notifications@github.com Sent: Wednesday, August 15, 2018 12:06 AM To: eranpeer/FakeIt FakeIt@noreply.github.com Cc: eranpeer eranpe@gmail.com; Comment comment@noreply.github.com Subject: Re: [eranpeer/FakeIt] Issue with mock with shared_ptr used inself-invocation (#153)

Good to know that helped. I was struggling to fix it, since I don't understand the code very well. I'll get there eventually.

If the test pass, it is definitely fixed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413112898, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8gn4vPhSOKjftJU728FkrcCBATJouks5uQ8h9gaJpZM4V6Vv5 .[image: https://github.com/notifications/beacon/ACc8gsjSGMl_tnycXhfm77OqWo2lEXR2ks5uQ8h9gaJpZM4V6Vv5.gif]

-- Eran 1-424-2504000

[ericlemes]

I like getShared, but to be honest, I didn't see the code yet. I'm going on holidays, back next Tuesday.

I had a similar request from an user which is a getUnique and in the whole smart pointer world a "ReturnRef" for the When would be ideal. GMock has that. The idea is just returning a std::move, so it forces to use the move constructor instead of copy constructor which solves the problem of mocking methods that returns object from classes without copy constructor. AlwaysDo solves the problem, but it is not intuitive.

These are just ideas and completely out of the scope of this issue.

On Thu, Aug 16, 2018, 05:39 eranpeer notifications@github.com wrote:

I fixed it!. I added a new api: mock.getShared() that returns a shared_ptr of the mock instance. It is like mock.get() but it returns a shared_ptr. Since fakeit is the owner of the mock instance, any attempt to wrap it with a shared_ptr will result in the shared_ptr deleting the instance when the reference count is 0. in order to avoid that simply obtain the shared_ptr from the mock.getShared() new api:

class ISomeInterface3 { public: virtual ~ISomeInterface3() {}

virtual void methodWithSomeSharedPointer(std::shared_ptr listener) = 0; };

void production_shared_ptr_mock_used_in_invocation() { std::shared_ptr pMockInstance; Mock mock; fakeit::Fake(Dtor(mock)); fakeit::Fake(Method(mock, methodWithSomeSharedPointer));

pMockInstance = mock.getShared(); pMockInstance->methodWithSomeSharedPointer(pMockInstance);

pMockInstance = nullptr; }

I think I can solve it in another way without introducing a new API. Let me know what you think about this approach, maybe a better name for getShared()?

On Wed, Aug 15, 2018 at 12:14 AM Eran Pe'er eranpe@gmail.com wrote:

I’ve just noticed that the build is broken on master. It is my mistake. I assumed that it will compile on Linux-gcc. I was wrong. Working on it now.

Sent from Mail https://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10

From: Eric Lemes notifications@github.com Sent: Wednesday, August 15, 2018 12:06 AM To: eranpeer/FakeIt FakeIt@noreply.github.com Cc: eranpeer eranpe@gmail.com; Comment comment@noreply.github.com Subject: Re: [eranpeer/FakeIt] Issue with mock with shared_ptr used inself-invocation (#153)

Good to know that helped. I was struggling to fix it, since I don't understand the code very well. I'll get there eventually.

If the test pass, it is definitely fixed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413112898, or mute the thread < https://github.com/notifications/unsubscribe-auth/ACc8gn4vPhSOKjftJU728FkrcCBATJouks5uQ8h9gaJpZM4V6Vv5

.[image:

https://github.com/notifications/beacon/ACc8gsjSGMl_tnycXhfm77OqWo2lEXR2ks5uQ8h9gaJpZM4V6Vv5.gif ]

-- Eran 1-424-2504000

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413423842, or mute the thread https://github.com/notifications/unsubscribe-auth/ABsadOKEl8Y95BADp1BIgqkm74JAzMR8ks5uRPd_gaJpZM4V6Vv5 .

[eranpeer]

OK, I found a better way to fix it. No new API. your version of the test should pass now! I reverted the new API.

On Wed, Aug 15, 2018 at 11:47 PM Eric Lemes notifications@github.com wrote:

I like getShared, but to be honest, I didn't see the code yet. I'm going on holidays, back next Tuesday.

I had a similar request from an user which is a getUnique and in the whole smart pointer world a "ReturnRef" for the When would be ideal. GMock has that. The idea is just returning a std::move, so it forces to use the move constructor instead of copy constructor which solves the problem of mocking methods that returns object from classes without copy constructor. AlwaysDo solves the problem, but it is not intuitive.

These are just ideas and completely out of the scope of this issue.

On Thu, Aug 16, 2018, 05:39 eranpeer notifications@github.com wrote:

I fixed it!. I added a new api: mock.getShared() that returns a shared_ptr of the mock instance. It is like mock.get() but it returns a shared_ptr. Since fakeit is the owner of the mock instance, any attempt to wrap it with a shared_ptr will result in the shared_ptr deleting the instance when the reference count is 0. in order to avoid that simply obtain the shared_ptr from the mock.getShared() new api:

class ISomeInterface3 { public: virtual ~ISomeInterface3() {}

virtual void methodWithSomeSharedPointer(std::shared_ptr listener) = 0; };

void production_shared_ptr_mock_used_in_invocation() { std::shared_ptr pMockInstance; Mock mock; fakeit::Fake(Dtor(mock)); fakeit::Fake(Method(mock, methodWithSomeSharedPointer));

pMockInstance = mock.getShared(); pMockInstance->methodWithSomeSharedPointer(pMockInstance);

pMockInstance = nullptr; }

I think I can solve it in another way without introducing a new API. Let me know what you think about this approach, maybe a better name for getShared()?

On Wed, Aug 15, 2018 at 12:14 AM Eran Pe'er eranpe@gmail.com wrote:

I’ve just noticed that the build is broken on master. It is my mistake. I assumed that it will compile on Linux-gcc. I was wrong. Working on it now.

Sent from Mail https://go.microsoft.com/fwlink/?LinkId=550986 for Windows 10

From: Eric Lemes notifications@github.com Sent: Wednesday, August 15, 2018 12:06 AM To: eranpeer/FakeIt FakeIt@noreply.github.com Cc: eranpeer eranpe@gmail.com; Comment <comment@noreply.github.com

Subject: Re: [eranpeer/FakeIt] Issue with mock with shared_ptr used inself-invocation (#153)

Good to know that helped. I was struggling to fix it, since I don't understand the code very well. I'll get there eventually.

If the test pass, it is definitely fixed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413112898 , or mute the thread <

https://github.com/notifications/unsubscribe-auth/ACc8gn4vPhSOKjftJU728FkrcCBATJouks5uQ8h9gaJpZM4V6Vv5

.[image:

https://github.com/notifications/beacon/ACc8gsjSGMl_tnycXhfm77OqWo2lEXR2ks5uQ8h9gaJpZM4V6Vv5.gif ]

-- Eran 1-424-2504000

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413423842, or mute the thread < https://github.com/notifications/unsubscribe-auth/ABsadOKEl8Y95BADp1BIgqkm74JAzMR8ks5uRPd_gaJpZM4V6Vv5

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eranpeer/FakeIt/issues/153#issuecomment-413443393, or mute the thread https://github.com/notifications/unsubscribe-auth/ACc8gpooD_ahFNKMFL8lFMDvdhXAhtloks5uRRWJgaJpZM4V6Vv5 .

-- Eran 1-424-2504000

[helmesjo]

Should this be closed?

[VirtualTim]

@helmesjo it looks like this code is currently still commented out: https://github.com/eranpeer/FakeIt/commit/19bd30adcf8e1ce4f251d4b77d0abcc1ca9424ab.

[FranckRJ]

It's intended as far as I understand, it's not necessary anymore.

[FranckRJ]

Because it look like it's fixed (cannot reproduce) I'll close it.