{"action":"opened","issue":{"url":"https://api.github.com/repos/OpenMAMA/OpenMAMA/issues/114","labels_url":"https://api.github.com/repos/OpenMAMA/OpenMAMA/issues/114/labels{/name}","comments_url":"https://api.github.com/repos/OpenMAMA/OpenMAMA/issues/114/comments","events_url":"https://api.github.com/repos/OpenMAMA/OpenMAMA/issues/114/events","html_url":"https://github.com/OpenMAMA/OpenMAMA/issues/114","id":130464754,"number":114,"title":"[MAMACPP] MamaSubscription destructor does not completely disconnect application from MamaSubscriptionImpl.","user":{"login":"cjwmorgan-sol-sys","id":17009318,"avatar_url":"https://avatars.githubusercontent.com/u/17009318?v=3","gravatar_id":"","url":"https://api.github.com/users/cjwmorgan-sol-sys","html_url":"https://github.com/cjwmorgan-sol-sys","followers_url":"https://api.github.com/users/cjwmorgan-sol-sys/followers","following_url":"https://api.github.com/users/cjwmorgan-sol-sys/following{/other_user}","gists_url":"https://api.github.com/users/cjwmorgan-sol-sys/gists{/gist_id}","starred_url":"https://api.github.com/users/cjwmorgan-sol-sys/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cjwmorgan-sol-sys/subscriptions","organizations_url":"https://api.github.com/users/cjwmorgan-sol-sys/orgs","repos_url":"https://api.github.com/users/cjwmorgan-sol-sys/repos","events_url":"https://api.github.com/users/cjwmorgan-sol-sys/events{/privacy}","received_events_url":"https://api.github.com/users/cjwmorgan-sol-sys/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-02-01T19:51:01Z","updated_at":"2016-02-01T19:51:01Z","closed_at":null,"body":"### OpenMAMA Version\r\nVersion: 2.3.3\r\n\r\n###  Description:\r\nOn the openmama Developer Wiki under Bridge Documentation for myMiddlewareBridgeMamaSubscription_destroy () it states “…the destroy method adds an event to the queue to invokes the destroy callback, which notifies the client application of the successful destroy of the subscription.”\r\n\r\nWhen an application destructs a MamaSubscription object, the application onDestroy is called inline, and the MamaSubscription object is freed, but the corresponding MamaSubscriptionImpl object is not freed.  So far so good – it is the MamaSubscriptionImpl object pointer that is the closure to the C-layer and this is still valid, unfreed memory.  However, at this point the MamaSubscriptionImpl object’s mCallback pointer still points to the application’s MamaSubscriptionCallback object.  The application may have freed this memory since MamaSubscriptionCallback::onDestroy() has been called.\r\n\r\n### Example Scenario :\r\n\r\nIf we look at MamdaSubscription, it in fact does not implement MamaSubscriptionCallback::onDestroy at all.  It will always destroy the callback (MamdaSubscription::mImpl) immediately after destroying the underlying MamaSubscription.  This happens to occur after onDestroy is called since it is called inline with MamaSubscription::~MamaSubscription.  But nevertheless, it is freed immediately after onDestroy is called inline with the call to MamaSubscription::~MamaSubscription().\r\n\r\nNow, sometime later, the C-layer onDestroy is called.  This calls the static MamaSubscription::onSubscriptionDestroy which calls MamaSubscriptionImpl::InvokeDestroy().  This checks if MamaSubscriptionImpl::mCallback is NULL (it is not – in the case of a MamdaSubscription it is still pointing to a freed MamdaSubscriptionImpl object) and tries to invoke the onDestroy() method.  If the vtable pointer is no longer valid, a core here is quite likely.\r\n\r\n### Potential Fix :\r\n\r\nEven if a core does not occur, the application would see onDestroy() twice – once inline with when the destroy was started and a second time when the lower layer destroy was completed.  It seems the design intent here is to decouple the MamaSubscriptionImpl from everything above it when the MamaSubscription is destroyed.  To do this, it seems the right thing to do in the C++ layer would be to set MamaSubscriptionImpl::mCallback to NULL when setting MamaSubscriptionImpl::mFreed to true (or alternatively include mFreed in the tests that check mCallback == NULL).\r\n\r\n\r\nHere is a patch that includes mFreed the test that mCallback == NULL.\r\n\r\n[MamaSubscription.cpp.patch.txt](https://github.com/OpenMAMA/OpenMAMA/files/112898/MamaSubscription.cpp.patch.txt)\r\n\r\n"}}