Re: Openmama Documentation for subscription onDestroy


Dmitri Fedorov <dfedorov.solace@...>
 

Hi Frank,

I'm going to echo Duane: this is a very straightforward and low risk fix, and it addresses a severe issue.

We'll make sure we follow a proper process from now on, but for this one time could you please pick it up from the patch file attached to the issue (114)?

Thank you
Dmitri


Regards,
Dmitri Fedorov
Software Architect
Solace Systems, Inc.
Ottawa, ON Canada

Solace Systems accepts no liability for the content of this email, or for the consequences of any actions taken on the basis of the information provided, unless that information is subsequently confirmed in writing. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Solace Systems.

On 9 March 2016 at 16:31, Frank Quinn <fquinn.ni@...> wrote:

Hi Dmitri,


It’s not tagged for the 2.4.0 milestone as of today... why? Is it a showstopper for you guys? If so we can see if we can get it in.


To provide some context, when preparing release cuts, we look for any showstopper bugs and pull requests as well as any major features that are part of a release. Because this issue was a patch masquerading as an "issue" rather than a "pull request", it didn't make the cut at the time.


Cheers,

Frank


On Wed, Mar 9, 2016 at 4:38 PM, Dmitri Fedorov <dfedorov.solace@...> wrote:
Gents, are you taking this path for 2.4?

Thank you
Dmitri Fedorov


Regards,
Dmitri Fedorov
Software Architect
Solace Systems, Inc.
Ottawa, ON Canada

Solace Systems accepts no liability for the content of this email, or for the consequences of any actions taken on the basis of the information provided, unless that information is subsequently confirmed in writing. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Solace Systems.

On Mon, Feb 1, 2016 at 3:18 PM, Christopher Morgan <Christopher.Morgan@...> wrote:

I’ve raised an issue to cover this on github.

 

Here is a link to the issue:

https://github.com/OpenMAMA/OpenMAMA/issues/114

 

Chris Morgan

 

 

From: Alpert, Reed [mailto:reed.alpert@...]
Sent: February-01-16 10:48 AM
To: Duane Pauls <Duane.Pauls@...>; Christopher Morgan <Christopher.Morgan@...>; openmama-dev@...
Subject: RE: Openmama Documentation for subscription onDestroy

 

I think the app needs to call sub->destroy(), then wait for the onDestroy(), and then ‘delete sub’.

 

onDestroy() is a completion event – once you get it you won’t hear from that subscription again (no callbacks of any kind).

 

In order for an app to call C++ or Java ‘delete sub’ w/o sub->destroy the destructor needs to completely disconnect the app from the impl (which as Duane & Chris point out it does not).

In that case the inline onDestroy() does have value in that the app knows it can safely destroy any of its data structures.

 

Thanks,

 

Reed.

 


Reed Alpert | Corporate & Investment Bank | Market Data Services | J.P. Morgan | 4 Metrotech Center, 23rd Floor, Brooklyn, NY 11245 | T: 718.242.5198  | M: 917.414.4613 | reed.alpert@...

Alternate Contact:  CIB PIM Trading Technology Solutions NA | CIB_PIM_Trading_Technology_Solutions_NA@...

 

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Duane Pauls
Sent: Thursday, January 28, 2016 1:39 PM
To: Christopher Morgan; openmama-dev@...
Subject: Re: [Openmama-dev] Openmama Documentation for subscription onDestroy

 

I’ll add a bit of clarification to the problem with the CPP interface if the middleware bridge queues an event to call the onDestroy callback.

 

When 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.

 

If we look at MamdaSubscription as an example, 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().

 

Now, 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.

 

Even 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).

 

Should a bug be raised for this issue?  Or should a middleware bridge in fact NOT enqueue the onDestroy callback as stated in the OpenMAMA Bridge Documentation Wiki and instead invoke onDestroy inline?  If the intent is to always invoke the callback inline, it seems the benefit of having a callback in the first place is diminished.

 

Cheers,

Duane

 

From: Christopher Morgan
Sent: Thursday, January 28, 2016 10:30 AM
To:
openmama-dev@...
Subject: Openmama Documentation for subscription onDestroy

 

Hi,

 

I’m with solace dev for the openmama middle bridge and I have question about mamaSubscription onDestroy with the CPP openmama. On 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.” Our Bridge currently does this but in the CPP API for openmama the MamaSubscription seems to do an inline onDestroy as a part of the destructor. This does not work well with the enqueuerd onDestroy callback (potential for causing segmentation faults). I’ve also compared to the Qpid bridge which seems to do an inline onDestroy callback as a part their MiddlewareBridgeMamaSubscription_destroy(). Should the onDestroy callback be inline the MiddlewareBridgeMamaSubscription_destroy() like the Qpid to work with the CPP API? Or Is there a problem with the CPP API?

 

Chris Morgan

This communication is for informational purposes only. It is not intended as an offer or solicitation for the purchase or sale of any financial instrument or as an official confirmation of any transaction. All market prices, data and other information are not warranted as to completeness or accuracy and are subject to change without notice. Any comments or statements made herein do not necessarily reflect those of JPMorgan Chase & Co., its subsidiaries and affiliates (collectively, "JPMC"). This transmission may contain information that is proprietary, privileged, confidential and/or exempt from disclosure under applicable law. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the information contained herein (including any reliance thereon) is STRICTLY PROHIBITED. If you received this transmission in error, please immediately contact the sender and destroy the material in its entirety, whether in electronic or hard copy format. Although this transmission and any attachments are believed to be free of any virus or other defect that might affect any computer system into which it is received and opened, it is the responsibility of the recipient to ensure that it is virus free and no responsibility is accepted by JPMC for any loss or damage arising in any way from its use. Please note that any electronic communication that is conducted within or through JPMC's systems is subject to interception, monitoring, review, retention and external production in accordance with JPMC's policy and local laws, rules and regulations; may be stored or otherwise processed in countries other than the country in which you are located; and will be treated in accordance with JPMC policies and applicable laws and regulations. Please refer to http://www.jpmorgan.com/pages/disclosures for disclosures relating to European legal entities.


_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev



_______________________________________________
Openmama-dev mailing list
Openmama-dev@...
https://lists.openmama.org/mailman/listinfo/openmama-dev



Join Openmama-dev@lists.openmama.org to automatically receive all group messages.