Re: Questions about MamaStartCallback


Frank Quinn <fquinn@...>
 

Thanks Yury,

Yes that analysis and suggestion both sound correct. I just raised https://github.com/OpenMAMA/OpenMAMA/issues/319 for tracking so it will make it into the next release.

Cheers,
Frank


FRANK QUINN
Principal Engineer
Vela Trading Technologies

O. +44 289 568 0209 ext. 3592
fquinn@velatt.com

Adelaide Exchange Building, 2nd Floor, 24-26 Adelaide Street, Belfast, BT2 8GD
velatradingtech.com | @vela_tt

-----Original Message-----
From: Yury Batrakov [mailto:yury.batrakov@db.com]
Sent: 14 August 2017 13:21
To: openmama-users <openmama-users@lists.openmama.org>; openmama-dev <openmama-dev@lists.openmama.org>
Cc: Frank Quinn <fquinn@velatt.com>; Marc Alzina <marc.alzina@db.com>
Subject: Questions about MamaStartCallback

Classification: Public

Hi guys,

As everybody knows there is possibility to start MAMA bridge in background thread in C++ API: function Mama::startBackground (mamaBridge bridgeImpl, MamaStartCallback* cb) There are a couple of problems with this function though:
1. We can't ignore "MAMA startup finished" event setting the value for the second argument to nullptr - this can be fixed as a simple nullptr check in void MAMACALLTYPE stopCb (mama_status status, mamaBridge, void* closure) in mamacpp.cpp
2. There is race condition between thread calling Mama::stop and internal MAMA thread spawned by startBackground function. This race may lead to program crash.

Let me describe #2 - consider the following scenario:

We have a simple class MyMamaConnection

class MyMamaStartCallback : public MamaStartCallback {
void onStartComplete (Wombat::MamaStatus status) override {
// .... Some bridge specific callback code
}
};

class MyMamaConnection {
mamaBridge m_bridge;
MyMamaStartCallback m_callback;

MyMamaConnection() {
Mama::startBackground (m_bridge, &m_callback)
}

~MyMamaConnection() {
Mama::stop(m_bridge);
}
};

The crash occur in destructor because Mama::stop doesn't guarantee that no threads are still using m_callback pointer after this function exited. In our case MyMamaConnection's destructor destroys m_callback object while it is still in use by the thread created by Mama::startBackground.

Here are more details, file mama.c:

static void* mamaStartThread (void* closure) { ...
rval = mama_start (cb->mBridgeImpl); // Imagine user thread Thread_1 calling ~MyMamaConnection() from our example.

// Current thread may be suspended by OS at this point while Thread_1 continues its work and deletes the object pointed by closure ...

if (cb->mStopCallbackEx)

// cb->mClosure points to destroyed object.

cb->mStopCallbackEx (rval, cb->mBridgeImpl, cb->mClosure); ...
}

One of the possible solutions is to move this code from mama_close() to mama_stop():
// Get name of start background thread and destroy it
const char* threadName = wombatThread_getThreadName(middlewareLib->bridge->mStartBackgroundThread);
wombatThread_destroy (threadName);

Let me know if it's possible to have this fixed in the next release.


---
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and delete this e-mail. Any unauthorized copying, disclosure or distribution of the material in this e-mail is strictly forbidden.

Please refer to https://www.db.com/disclosures for additional EU corporate and regulatory disclosures and to http://www.db.com/unitedkingdom/content/privacy.htm for information about privacy.

The information contained in this message may be privileged and confidential and protected from disclosure. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to this message and deleting it from your computer. Thank you. Vela Trading Technologies LLC

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