Replacing RB-118

When run against MamaPublisherCS.exe, MamaInboxCS.exe throws a
AccessViolationException, and exits.

$ MamaInboxCS.exe -m qpid -tport sub
Starting Publisher with:
topic: MAMA_INBOUND_TOPIC
transport: sub

Error occurred: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Details:
System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
at Wombat.Mama.NativeMethods.mama_start(IntPtr bridgeImpl)
at Wombat.Mama.start(MamaBridge bridgeImpl)
at Wombat.MamaInboxCS.Run()
at Wombat.EntryPoint.Main(String[] args)

Based on the logging from MamaPublisherCS, it appears that a connection is
made, and some inbox requests are sent. However, shortly these do not appear to
be received by MamaInboxCS.

Publishing message to MAMA_TOPIC.
Publishing message to MAMA_TOPIC.
Received inbound msg. Sending response
Publishing message to inbox.
Received inbound msg. Sending response
Publishing message to inbox.
Received inbound msg. Sending response
Publishing message to inbox.

Similar errors are observed when running MamaInboxCS against mamapublisherc,
mamapublishercpp or MamaPublisherJava.

Other combinations of Publisher (C, C++, Java) with Inbox (C, C++, Java) behave
as expected.

It's the qpid-specific parts of this change:

a4a684e

mama_status
mamaMsg_updateVectorPrice (
mamaMsg msg,
const char* fname,
mama_fid_t fid,
//should be const mamaPrice priceList[]
=> const mamaPrice* priceList[], =<
mama_size_t numElements);

msgPayload_getPrice, msgPayload_getDateTime should take pointer to result as last parameter
Eg:
typedef mama_status
(msgPayload_getDateTime) (const msgPayload msg,
const char name,
mama_fid_t fid,
//should be mamaDateTime * result
=> mamaDateTime result); <=

Replacing BZ-41

When running a mamapublisher / mamasubscriber test, after a subscriber connects and then disconnects, you will receive the following:

Publishing message 16 to MAMA_TOPIC. msg=({{MdMsgStatus,2,0},{MdSeqNum,10,16},{PublisherTopic,10002,MAMA_TOPIC}}
Publishing message 17 to MAMA_TOPIC.ERROR amqp:connection:framing-error connection aborted
[0xb5809090:0] ERROR[-2] connection aborted
CONNECTION ERROR connection aborted
msg=({{MdMsgStatus,2,0},{MdSeqNum,10,17},{PublisherTopic,10002,MAMA_TOPIC}}
Publishing message 18 to MAMA_TOPIC.ERROR amqp:connection:framing-error connection aborted
[0x9c8aff8:0] ERROR[-2] connection aborted
msg=({{MdMsgStatus,2,0},{MdSeqNum,10,18},{PublisherTopic,10002,MAMA_TOPIC}}
Publishing message 19 to MAMA_TOPIC.CONNECTION ERROR connection aborted
CONNECTION ERROR connection aborted
msg=({{MdMsgStatus,2,0},{MdSeqNum,10,19},{PublisherTopic,10002,MAMA_TOPIC}}
Publishing message 20 to MAMA_TOPIC.CONNECTION ERROR connection aborted
msg=({{MdMsgStatus,2,0},{MdSeqNum,10,20},{PublisherTopic,10002,MAMA_TOPIC}}
Publishing message 21 to MAMA_TOPIC.CONNECTION ERROR connection aborted

This is by design because there was no known way with the qpid proton interface to tell whether a network event has occurred (in which case publish should keep trying) or whether the subscriber simply went away. This ticket is to determine if there is a better way to handle this, possibly with some sort of timeout / cleanup on connections which have gone away for whatever reason.

Replacing bz-121
Building OpenMAMA with clang emits the following warning indicating that the if statement will always execute to true:

WARNING:: common/c_cpp/src/c/MRSWLock.c:200:13: warning: comparison of address of 'lock->m_noLocksEvent' not equal to a null pointer is always true
[-Wtautological-pointer-compare]
                if(&lock->m_noLocksEvent != NULL)

This should be fixed. Patch attached to Bugzilla.

Replacing RB-126

Default x64 and Win32 builds of gtest-md are in different location.
And the search path in the unit-test MainUnitTestC does not reflect that.

Proposed solution for gtest-md 1.7.0:

I suggest to add the following paths to the property sheet of the project:

GTEST_HOME is where the gtest is extracted.

In Property Pages -> Configuration Properties -> Linker -> General -> Addition
Library Directories should add:
Win32|Debug: $(GTEST_HOME)\msvc\gtest-md\Debug
Win32|Release: $(GTEST_HOME)\msvc\gtest-md\Release
x64|Debug: $(GTEST_HOME)\msvc\x64\Debug
x64|Release: $(GTEST_HOME)\msvc\x64\Release

I also suggest to make sure in the Wiki for visual studio that people build the
gtest-md (or gtest) in the following specific way to avoid confusion:

Do not build with CMake. for VS use the solution in:
%GTEST_HOME%\msvc\gtest-md.sln

Create x64 configuration. In configuration Manager create new platform and copy
its configuration from Win32

In gtest-md Property Pages -> Configuration Properties -> C/C++ -> Code
Generation -> Runtime Library should be:

• For Debug: Multi-threaded Debug (/MTd)
• For Release: Multi-threaded (/MT)

Build all platforms and configuration of gtest-md

From Duane's bugzilla ticket:

Created attachment 218 [details]
Don't require write to socketpair to succeed if the socket is full.

This issue was originally raised on the developer mailing list, and the
suggestion was to raise a bug for this. Below I've cut 'n' pasted the
description from the original email. I've also submitted a patch as an
attachment.

From email:

I’ve been running the timer unit tests repeatedly for over a week with the
patch submitted in bug 220:
http://bugs.openmama.org/show_bug.cgi?id=220

MamaTimerTestCPP.ForTimer recently entered a busy loop + deadlock. After
debugging the problem, I don’t think the problem I’m seeing is specific to the
patched OpenMAMA. I’d like to open a quick discussion to see if my proposed
fix sounds reasonable.

I saw that the socket write in _addTimer (a new function in the patch, but the
same logic as the wwrite in the original OpenMAMA) was failing due to ‘EAGAIN’,
so _addTimer entered a busy loop continually trying to write to the socket that
was full. The reader is unable to read from the socket since the lock is being
held for the duration of the write, and the reader tries to take the lock
before reading. By calling ioctl(heapImpl->mSockPair[0], FIONREAD, buf), I
determined there were 423 bytes available for reading. It seems this is the
maximum number of records that can be written to a socket pair in Linux. See:
http://stackoverflow.com/questions/10899814/af-unix-socket-overhead

I’m thinking that to fix this, we can simply ignore EAGAIN (and for maximum
portability EWOULDBLOCK as well). If the socket is full, there are plenty of
records there to wake up the reader, so it isn’t necessary to add another
‘wake-up’ record. Since the lock was taken before the new timer was added and
continues to be held through the write, we can just release the lock knowing
the reader has something to wake it up.

Replacing bz-113

There appears to be an issue with building wMessageStats.c on Windows, which generates the following warning:

wMessageStats.c(618): warning C4013: 'strptime' undefined; assuming extern returning int

Windows doesn't actually have a native strptime implementation, so common actually includes one in the windows compatibility code (windows/strptime.c). For some reason this has been commented out of the wMessageStats.c code, and thus we hit this warning. I believe this will likely still operate as expected on 32 bit machines as long as int's and pointers are the same size, and the strptime code is actually compiled into the common libraries (which should be the case in both SCons and VS builds). It'll simply resolve the function during link time later.

It is less likely to operate correctly on a 64 bit machine, where int's and pointers are likely to have different sizes. If strptime returns a larger pointer value, it may get truncated, and everything will die horribly.

The fix seems fairly obvious, however more testing/investigation is needed to understand why the code was changed in the first place.

mamaBridgeImpl::mInternalDispatcher is not set to NULL on bridge creation which can lead to a crash if this variable is used.

Replacing RB-17

At present the mamaMsg_freeString call has no implementation, and won't
actually do anything if called. It is also confusing, as it may make API users
believe they own the memory returned by mamaMsg_toString, which may ultimately
lead to further undefined behaviour if they attempt to modify or free the
buffer.

The mamaMsg documentation should be updated to reflect the fact that freeString
is no longer supported, and references to it removed from the rest of the code
base. A note should also be added to toString to make it clear that the
returned buffer is owned by the message itself, not by the user.

There are some pesky macros defined in port.h (close, sleep etc) which causes problems when using other libraries (eg boost). We should be defining them as inline functions instead of using macros

Replacing bz-170

We have a report of a crash in MAMA when dereferencing a null pointer in
list_for_each(wList list, wListCallback cb, void* closure) from
/common/c_cpp/src/c/list.c, though so far we don't have a reliable recreation
of the issue.

From the analysis provided:

>  std::unique_ptr<Wombat::MamaTransport> tport_;
>
>  if (bridge_)  // NB. bridge_ is a mamaBridge* pointer
>  try
>  {
>     tport_.reset(new Wombat::MamaTransport);  // <-- the MamaTransport::MamaTransport constructor is successful
>     tport_->setTransportCallback(this);
>     tport_->create(tport_name, *bridge_);  // <-- this MamaTransport::create call fails and throws a MAMA exception
>   }
>   catch (const Wombat::MamaStatus& exc)
>   {
>     tport_.reset(nullptr);  //  <-- this calls MamaTransport::~MamaTransport, which triggers an unexpected crash in MAMA
>      //...
>     return false;
>   }
>   catch (...)
>   {
>     tport_.reset(nullptr);
>      //...
>     return false;
>   }

Reviewing the code within mama/c_cpp/src/c/transport.c, the following scenario
seems very likely:

(1) mamaTransport_create fails at some point before the call to function init

(2) self->mListeners is a null pointer (because, although
mamaTransport_allocate was successful, mListeners is assigned a list created by
list_create(sizeof (SubscriptionInfo)) in function init, which wasn't called -
see 1)

(3) mamaTransport_destroy(mamaTransport transport) is then called from the
MamaTransport::~MamaTransport destructor

(3.1) mamaTransport_destroy calls
self->mBridgeImpl->bridgeMamaTransportIsValid, which calls into the MAMA
middleware bridge and checks that the transport is valid
(3.2) the MAMA middleware bridge reports that the transport is valid
(3.3) then mamaTransport_destroy proceeds with a call to
mamaTransportImpl_clearTransportWithListeners(self)

(3.3.1) in mamaTransportImpl_clearTransportWithListeners, mRefreshTransport
is a null pointer (the initialization of mRefreshTransport would happen, if at
all, in mamaTransport_create at some point after function init is called, but
mamaTransport_create failed before the call to function init)
(3.3.2) mamaTransportImpl_clearTransportWithListeners then calls

> list_for_each(impl->mListeners,
> mamaTransportImpl_clearTransportCallback, NULL);

which causes a crash because mListeners is a null pointer (see 2).

Replacing bz-112
The Windows port of wthread_cond_wait passes a pointer to a LPCRITICAL_SECTION to both the LeaveCriticalSection and EnterCriticalSection functions. These should both be LPCRITICAL_SECTIONS themselves.

Building on Windows with VS results in the following warnings:

1>windows\port.c(135): warning C4047: 'function' : 'LPCRITICAL_SECTION' differs in levels of indirection from 'LPCRITICAL_SECTION *'
1>windows\port.c(135): warning C4024: 'LeaveCriticalSection' : different types for formal and actual parameter 1
1>windows\port.c(138): warning C4047: 'function' : 'LPCRITICAL_SECTION' differs in levels of indirection from 'LPCRITICAL_SECTION *'
1>windows\port.c(138): warning C4024: 'EnterCriticalSection' : different types for formal and actual parameter 1

xxxPayload_getMsg(const msgPayload msg, const char* name, mama_fid_t fid, msgPayload* result)

We are passing in a const msg and extracting its field. so, last arg should be const msgPayload*

Replacing BZ-38

The interface for these methods looks inconsistent with the others and causes difficulty in propagating results upwards from the payload implementation layer.

The current interface looks like this:

mamaMsg_getVectorPrice (
    const mamaMsg         msg,
    const char*           name,
    mama_fid_t            fid,
    const mamaPrice*      result,
    mama_size_t*          resultLen)

Whereas it should look more like this to enable the underlying interface to write to the result pointer provided which in this context is essentially an array of mamaPrices:

mamaMsg_getVectorPrice (
    const mamaMsg         msg,
    const char*           name,
    mama_fid_t            fid,
    const mamaPrice**     result,
    mama_size_t*          resultLen)

This would also align with all the other getVector functions, e.g.:

mamaMsg_getVectorF32 (
        const mamaMsg        msg,
        const char*          name,
        mama_fid_t           fid,
        const mama_f32_t**   result,
        mama_size_t*         resultLen)

Replacing BZ-193
When attempting to build OpenMAMA with scons it will fail to build when using multiple build types.
Specifically within the omama.conf

buildtype = 'all'

or

buildtype = 'dynamic,dynamic-debug'

The following error is generated:

Install file: "mama\c_cpp\src\examples\c\Makefile.sample" as "openmama_install_2.3.2\examples\mama\c\Makefile.sample"
scons: *** [openmama_install_2.3.2\examples\mama\c\Makefile.sample] AssertionError : Installing source ['mama\c_cpp\src\examples\c\Makefile.sample', 'mama\c_cpp\src\examples\c\Makefile.sample'] into target ['openmama_install_2.3.2\examples\mama\c\Makefile.sample']: target and source lists must have same length.
Traceback (most recent call last):
File "C:\Python27\Scripts..\Lib\site-packages\scons-2.3.0\SCons\Action.py", line 1062, in execute
result = self.execfunction(target=target, source=rsources, env=env)
File "C:\Python27\Scripts..\Lib\site-packages\scons-2.3.0\SCons\Tool\install.py", line 215, in installFunc
"Installing source %s into target %s: target and source lists must have same length."%(list(map(str, source)), list(map(str, target)))
AssertionError: Installing source ['mama\c_cpp\src\examples\c\Makefile.sample', 'mama\c_cpp\src\examples\c\Makefile.sample'] into target ['openmama_install_2.3.2\examples\mama\c\Makefile.sample']: target and source lists must have same length.
scons: building terminated because of errors.

Replacing bz-194

Proposing a patch to fix an issue observed on non x86 platforms where invalid dates are logged.
The current implementation employs a time structure (struct tm) but only uses the tm_sec field. When passing to a function used to convert to local time a cast is performed. This cast causes an issue when tested on IBM Power, as a result an invalid date string was being logged to screen.

The attached patch (to be applied to mama/c_cpp/src/c/log.c) avoids the cast, using a time_t to store seconds since epoch instead of the struct and has been tested to work on both x86 and Power.

replacing bz-174.

From Gary Molloy:

I've been testing the setupSubscription and createSubscription methods found in
MamaSubscription.java. Key difference between the two is that setupSubscription
is used when a subscription is created but not activated yet where
createSubscription creates and activates the subscription.

In the setupSubscription, the closure is passed into the JNI layer instead of
saving it in the java layer like createSubscription. Without saving this
closure in the java layer, the value doesn't seem to be passed back properly
from the JNI layer causing issues.

Is this a typo in MamaSubscription.java or is the JNI layer supposed to pass
the closure back to the JAVA layer once the subscription is activated?

Replacing RB-172

We have just come across this little oddity and wonder if anyone can throw any
light on what is supposed to be happening

I notice from the source to the C# implementation of Open Mama 2.3.0, that the
following code:

        public static void open ()
        {
            initGetVersionWrapper();
            MamaWrapper.CheckResultCode(NativeMethods.mama_open());
            Interlocked.Increment(ref mMamaopen);
        }

increments the mMamaopen flag, whereas the code:

        public static void openWithProperties (string path, string filename)
        {
            initGetVersionWrapper();
            MamaWrapper.CheckResultCode (NativeMethods.mama_openWithProperties
            (path, filename));
        }

does not. The transport's destroy() method does nothing unless mMamaopen is
non-zero, and that is causing me problems when I try to shutdown an application
that uses openWithProperties. Making an additional call to open() gets round
the problem but doesn't really seem to be the right thing to do. Assuming it is
necessary should I expect to call open() before or after openWithProperties()

bool MamaMsg::setNewBuffer (
void* buffer,
mama_size_t size)
{
int result = 0;
// should be MAMA_STATUS_OK == ...
=> if (MAMA_STATUS_OK != (mamaTry (mamaMsg_setNewBuffer ( <=
mMsg, buffer, size))))
{
return result;
}
return result != 0;
}

Also, the return value is incorrect

from Alireza Assadzadeh:

Destroying throttled market data subscriptions and transport crashes
application due to seg fault.

The scenario is the following:

1. There are market data subscriptions that are throttled due to high number of
   subscriptions and therefore several subscriptions are in the activating state
   (i.e. MAMA_SUBSCRIPTION_ACTIVATING).

2. While the subscriptions creation is being throttled, the application
   destroys subscriptions (via mamaSubscrioption_destroyEx)

3. Before all the subscriptions are destroyed, the application destroys the
   transport (via mamaTransport_destroy)

After a few milliseconds the application crashes due to seg faults. The seg
fault may occur in different place (since it is believed the root cause is use
after free of a pointer). The most directly useful seg fauls is in
wombatThrottle_removeAction called from mamaSubscription_deactivate. See below
for valgrind output.

The problem is easily reproducible with large number of market data
subscriptions.

Let me know if you need sample application to reproduce the issue. Although we
used our internal test tools to produce this problem, I believe a modified
version of mamalistenc sample application with large number of subscriptions
could also be used to produce the problem.

Below line numbers for OpenMAMA are based on OpenMAMA 2.3.1 release.

Valgrind output

$ valgrind subscriberApp
[snip]
==17016== Thread 4:
==17016== Invalid read of size 8
==17016==    at 0x4F24051: wombatThrottle_removeAction (throttle.c:392)
==17016==    by 0x4F1E786: mamaSubscription_deactivate (subscription.c:2601)
==17016==    by 0x4F1E84F: mamaSubscription_destroy (subscription.c:2733)
==17016==    by 0x4F1E8FB: mamaSubscription_DestroyThroughQueueCB
(subscription.c:1546)
==17016==    by 0x4F31146: wombatQueue_dispatchInt (queue.c:306)
==17016==    by 0x7F78A51: solaceBridgeMamaQueue_dispatch (queue.c:306)
==17016==    by 0x4F0C25E: mama_start (mama.c:1338)
==17016==    by 0x4F0CA0E: mamaStartThread (mama.c:1366)
==17016==    by 0x321DA079D0: start_thread (in /lib64/libpthread-2.12.so)
==17016==    by 0x321CEE8B7C: clone (in /lib64/libc-2.12.so)
==17016==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==17016==.
==17016==.
==17016== Process terminating with default action of signal 11 (SIGSEGV):
dumping core
==17016==  Access not within mapped region at address 0x0
==17016==    at 0x4F24051: wombatThrottle_removeAction (throttle.c:392)
==17016==    by 0x4F1E786: mamaSubscription_deactivate (subscription.c:2601)
==17016==    by 0x4F1E84F: mamaSubscription_destroy (subscription.c:2733)
==17016==    by 0x4F1E8FB: mamaSubscription_DestroyThroughQueueCB
(subscription.c:1546)
==17016==    by 0x4F31146: wombatQueue_dispatchInt (queue.c:306)
==17016==    by 0x7F78A51: solaceBridgeMamaQueue_dispatch (queue.c:306)
==17016==    by 0x4F0C25E: mama_start (mama.c:1338)
==17016==    by 0x4F0CA0E: mamaStartThread (mama.c:1366)
==17016==    by 0x321DA079D0: start_thread (in /lib64/libpthread-2.12.so)
==17016==    by 0x321CEE8B7C: clone (in /lib64/libc-2.12.so)
==17016==  If you believe this happened as a result of a stack
==17016==  overflow in your program's main thread (unlikely but
==17016==  possible), you can try to increase the size of the
==17016==  main thread stack using the --main-stacksize= flag.
==17016==  The main thread stack size used in this run was 10485760.
==17016==.
==17016== HEAP SUMMARY:
==17016==     in use at exit: 1,099,936,144 bytes in 7,030,473 blocks
==17016==   total heap usage: 12,807,744 allocs, 5,777,271 frees, 1,950,556,698
bytes allocated
==17016==.
==17016== LEAK SUMMARY:
==17016==    definitely lost: 1,182 bytes in 102 blocks
==17016==    indirectly lost: 0 bytes in 0 blocks
==17016==      possibly lost: 36,407,723 bytes in 909,440 blocks
==17016==    still reachable: 1,063,527,239 bytes in 6,120,931 blocks
==17016==         suppressed: 0 bytes in 0 blocks
==17016== Rerun with --leak-check=full to see details of leaked memory
==17016==.
==17016== For counts of detected and suppressed errors, rerun with: -v
==17016== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 12 from 9)
Killed
$

Suspected Root Cause

• The mama_transportDestroy has been called and its throttle object for
  initials has been destroyed and nullified. Later on in mama_transportDestroy.
  the whole transport object will be freed.
• On the other hand, subscriptions are being deactivated that use this
  transport. These subscription destroys and hence deactivation were queued
  earlier as part of the call to mamaSubscrioption_destroyEx.
  Now, as part of the mamaSubscription_deactivate the throttle is being
  accessed for subscriptions that are waiting on throttle (these are the
  subscriptions in MAMA_SUBSCRIPTION_ACTIVATING state).

So the issue occurs when subscription throttle has throttled subscriptions and
then the transport is deleted.

Note that in mamaSubscription_deactivate the code has a null check for throttle
when locking the throttle, but there is no NULL checks in or before calling
wombatThrottle_removeAction which uses the throttle in the
MAMA_SUBSCRIPTION_ACTIVATING case.

Additional investigation

Other than the case of throttle in the transport becoming null, there is the
possibility that the subscription transport pointer becomes invalid when the
transport has been destroyed and freed completely in mamaTransport_destroy.

In this case the subscription transport pointer is invalid but is used in
mamaSubscription_deactivate. The mamaTransport_destroy has code at the
beginning of the function to inform various objects that use the transport that
the transport is being destroyed, for example by clearing their pointer to the
transport. Specifically, the mamaTransportImpl_clearTransportWithListeners
clears the transport pointer for some of the subscriptions via their transport
listener list (i.e. mListeners). However, I think there are also subscriptions
that don't have listeners yet (assuming these are the ones in
MAMA_SUBSCRIPTION_ACTIVATING
state). These subscriptions are not getting their transport pointer cleared.

Assuming the investigation above is correct, here are some thoughts (not
exhaustive)on how this issue can be fixed in MAMAC:

(1) Ensure the transport destroy is clearing the subscription's transport
pointer
for subscriptions in MAMA_SUBSCRIPTION_ACTIVATING state (and possible other
states) or
(2) Some other mechanism to synchronize the transport destroy and
subscription destroys.

Also the mamaTransport (particularly struct transportImpl_) does not maintain a
list of subscriptions, so if option (1) is chosen, the list of subscriptions on
a transport needs to be maintained.

Bridges are expected to allocate memory for some functions (toString, getReplyHandle etc) but there is no hook to free the memory. Since each bridge is allowed to use custom memory allocation, there should be a hook from mamaMsg_freeString back to the bridge. Also, currently mamaMsg_freeString is a no-op

Replacing RB-150

The SCons build is not setting SONAME for the shared libs (this was set
correctly with the autotools build)

e.g.

$ objdump -p ./objdir/mama/c_cpp/src/c/.libs/libmama.so |grep SONAME

doesn't give any output. SONAME is really important when packaging.

It appears correctly for 2.2.2.1:

$ objdump -p /usr/lib/x86_64-linux-gnu/libmama.so.0 | grep SONAME
SONAME libmama.so.0

Replacing RB-152

The vast majority of OpenMAMA's example applications make use of the same set
of command line options. However, each also reimplements command line parsing.
This leads to errors, inconsistency, and unfortunate levels of duplication.

We'd like to implement a generic toolset for command line parsing that can be
included in all example applications, and which will provide a standard
interface for each application. It should also allow extension to include
commands specific to an application.

Examples of common command line arguments:

-m
-tport
-S
-s
-f

Replacing RB-125

Payload and Middleware unit-test are missing from Visual Studio.

As per comments and documentation, mamaQueue_timedDispatch is supposed to keep dispatching until the given timeout has elapsed. But, the current bridge implementations use wombatQueue_timedDispatch which dispatches only one message. So, EITHER wombatQueue_timedDispatch should be changed to keep dispatching till timeout OR another helper function should be provided to do this (as this functionality is used across bridges)

replacing bz-203
Java MamaPublisher doesn't seem to have any implementation for destroy() or finalize().

Replacing bz-186

from Sam Wilson:

I'm tracking down a memory leak in our bridge, and I am trying to
understand why mamaMsg_setMsgBuffer doesn't set the owner flag in the
message. When the queue destroys the reusable message, it doesn't clean
up the payload.

MamaTransport::~MamaTransport (void)
{
if (mPimpl)
{
if ( mamaInternal_getCatchCallbackExceptions() && mPimpl->mCallback)
{
delete mPimpl->mCallback;
}
if (mPimpl->mDeleteCTransport)
{
=>mamaTry (mamaTransport_destroy (mTransport)); <=
}
delete mPimpl;
mPimpl = NULL;
}
}

We should just log and not throw an exception.

bridgeMamaPublisherSendReplyToInbox => request is void* instead of mamaMsg

typedef mama_status (*bridgeMamaPublisher_sendReplyToInbox)(publisherBridge publisher,
=>void * request, <=
mamaMsg reply);

This is needless as the request is a mamaMsg

Replacing bz-195
In MamaTransport.cpp the following status codes are missing:

• MAMA_TRANSPORT_CONNECT_FAILED
• MAMA_TRANSPORT_WRITE_QUEUE_LOW_WATER_MARK
• MAMA_TRANSPORT_WRITE_QUEUE_HIGH_WATER_MARK

Replacing BZ-18

At present, capture replay doesn't send a book clear message when it rewinds the playback file. While the data will continue to flow, it will look incorrect for orderbooks.

In order to resolve, we should send a book clear message to clients of book subscriptions whenever they rewind.

We may also want to do the same for regular subscriptions.

Details on mail thread.

Original poster has asked if there is a mechanism by which they can check that the data type of a field being published matches that stored in the dictionary - they are looking for an option which allows them to implement this across their publishers, without having to trust developers to implement checks themselves. after some discussion a general agreement that baking this into payloads/bridges isn't the right place was made, and that it should go into the C layer (so other languages don't need changed). The options ultimately are either within mamaMsg add/update* calls, or within the mamaPublisher.send, with pros and cons to each.

Ultimately the proposed solution would be to add a hook to mamaPublisher.send which can then be used by client libraries to perform whatever checks/manipulations they require.

Replacing bz-197

Proposing a patch to increase cross platform portability.
The current implementation for controlling locks uses x86 specific assembly.

The following patch (to be applied to common/c_cpp/src/c/linux/wInterlocked.h) uses GNU built-ins instead and has been used in a port to IBM Power.

On Windows closeSharedLib calls FreeLibrary, which returns non-zero on success, but closeSharedLib returns 0 on success. Just need to flip the return value from FreeLibrary. Currently this does no harm, but the plugin code prints an error when unloading a plugin. Will submit a patch for this.

We should identify a way to take this out of source control and make its creation (as opposed to its modification) part of the build process for all platforms.

Replacing RB-8

mamasubscriberc produces:

mamasubscriberc: Recieved msg.
MessageNo 10001 U32 32
PublisherTopic 10002 STRING MAMA

MamaSubscriberJava:

Received msg:
MessageNo | 10001 | U32 | 4
PublisherTopic | 10002 | STRING | MAMA
Reveived msg: Topic=MAMA, Sequence Nunber=5

Number (from Sequence Nunber above) is also spelt wrong with
MamaSubscriberJava.

The Java example should produce the same output as the C/C++ and .NET examples.
In this case the Java output appears easier to parse, but it is likely better
to stick to the precedent established in C.

Replacing bz-11
There appears to be a memory leak in the mamaMsg_detach method, where we create a copy of the detached message payload (msg->mPayload), then immediately overwrite the pointer to the original.

mamaMsg_detach (mamaMsg msg)
{  
    mamaMsgImpl*    impl    =   (mamaMsgImpl*)msg;
    msgPayload      payload = NULL;
...
    /* Copy the payload */
    if (MAMA_STATUS_OK != (status =
       (msg->mPayloadBridge->msgPayloadCopy (impl->mPayload,
                                             &payload))))
    {
        mama_log(MAMA_LOG_LEVEL_ERROR,
                 "mamaMsg_detach() Failed. "
                 "Could not copy native payload [%d]", status);
        return status;
    }

    msg->mPayload = payload;

The memory isn't immediately leaked at this point, as a reference is maintained to the original payload in the msg->mPayloads array. However, when mamaMsg_destroy is called the mPayload is destroyed, but the original remains.

Running under Valgrind we can see there is a memory leak associated with mamaMsgImpl_setMessageBuffer, which is where our payload value originates:

==53201==
==53201== 284,160 (7,680 direct, 276,480 indirect) bytes in 120 blocks are definitely lost in loss > record 99 of 106
==53201== at 0x4C2677B: calloc (vg_replace_malloc.c:593)
==53201== by 0x9365F9F: ???
==53201== by 0x4E5D4E4: mamaMsgImpl_setMsgBuffer (msg.c:543)
==53201== by 0x80E2282: ???
==53201== by 0x83256DF: ???
==53201== by 0x8303590: ???
==53201== by 0x80E1335: ???
==53201== by 0x4E57B5E: mama_start (mama.c:1324)
==53201== by 0x404FAC: main (mamalistenc.c:363)
==53201==
==53201== 333,384 (64 direct, 333,320 indirect) bytes in 1 blocks are definitely lost in loss > record 100 of 106
==53201== at 0x4C279EE: malloc (vg_replace_malloc.c:270)
==53201== by 0x957446B: ???
==53201== by 0x957CBDB: ???
==53201== by 0x9365FC4: ???
==53201== by 0x4E5D4E4: mamaMsgImpl_setMsgBuffer (msg.c:543)
==53201== by 0x80E2646: ???
==53201== by 0x83089AD: ???
==53201== by 0x8325659: ???
==53201== by 0x8303590: ???
==53201== by 0x80E1335: ???
==53201== by 0x4E57B5E: mama_start (mama.c:1324)
==53201== by 0x4052F0: main (mamalistenc.c:648)
==53201==

(Note: Valgrind can't create a full stack trace once a library has been closed, hence the big holes...)

In Windows only.
putenv() works differently on Windows - if the value is empty it removes the key from the env.
When gmtime is called it works differently if the TZ var is empty or missing.
Solution is to set TZ to UTC rather than empty in port.c
Patch and unit test to be submitted.

mamaMsg_copy does not check for self copy

Replacing BZ-141

There are a number of issues with the currently generated JavaDocs files from OpenMAMA. Specifically:

• Header and footer image cannot be found.
• Content needs update to reflect name 'OpenMAMA' rather than MAMA.

Most of these changes should be straightforward, and can be made to the mama/jni/SConscript and mama/jni/SConscript.win files.

Replacing BZ-39

Most of the implementation has been done before so this should simply be a case of cleaning up, but it wasn't completed because of #62 and #63

Theoretically it should be possible to test these before the MAMA changes are even made using the unit test framework already existing but with new functionality added to specifically handle date time / price validation.

During creation of the default queue, properties subsystem is accessed and this always prints an error/warning if we use a custom properties file.

creating this issue to check what workflows are available here.

msgPayload_unSerialize prototype incorrect (takes in void** instead of void*)

typedef mama_status
(msgPayload_unSerialize) (const msgPayload msg,
=>const void* buffer, <=
mama_size_t bufferLength);

This is an input buffer and the only function using this is mamaMsg_setNewBuffer which calls the above function and passes a void* buffer as a void**

Replacing BZ-37

The interface for these methods looks inconsistent with the others and causes difficulty in propagating results upwards from the payload implementation layer.

The current interface looks like this:

mamaMsg_getVectorDateTime (
    const mamaMsg         msg,
    const char*           name,
    mama_fid_t            fid,
    const mamaDateTime*   result,
    mama_size_t*          resultLen)

Whereas it should look more like this to enable the underlying interface to write to the result pointer provided which in this context is essentially an array of mamaDateTimes:

mamaMsg_getVectorDateTime (
    const mamaMsg         msg,
    const char*           name,
    mama_fid_t            fid,
    const mamaDateTime**  result,
    mama_size_t*          resultLen)

This would also align with all the other getVector functions, e.g.:

mamaMsg_getVectorF32 (
        const mamaMsg        msg,
        const char*          name,
        mama_fid_t           fid,
        const mama_f32_t**   result,
        mama_size_t*         resultLen)

Replacing RB-117

Java dump file.

When running the Java implementation of MamaListen, there is an intermittent
cores during shutdown when processing multiple (3 or more) symbols. This can be
reproduced against capturereplayc.exe using the sample data and dictionary. It
occurs when passing either multiple subscriptions (-s) or passing a symbol file
(-f), though not consistently.

$ java com.wombat.mama.examples.MamaListen -m qpid -tport sub -S TEST -s DE000CM95AU4.EUR.XPAR -s PTBIZJYE0064.EUR.ENXL -s NL0009906148.EUR.XAMS -shutdown 60
...
wQuoteSeqNum | 441 | I32 | 215618
wQuoteCount | 1034 | I32 | 1771
wLinkId | 2550 | I64 | 0
wOrigMsgType | 5001 | STRING | 140
MamaSendTime | 16 | STRING | 21:59:46.664
Subscription destroyed
Subscription destroyed
Subscription destroyed

A fatal error has been detected by the Java Runtime Environment:

EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x73f255d7, pid=2316, tid=2440

JRE version: Java(TM) SE Runtime Environment (8.0_05-b13) (build 1.8.0_05-b13)
Java VM: Java HotSpot(TM) Client VM (25.5-b02 mixed mode windows-x86 )
Problematic frame:
C [libmamaqpidimplmd.dll+0x55d7] qpidBridgeMamaTransportImpl_queueCallback+0x27

Failed to write core dump. Minidumps are not enabled by default on client versions of Windows

An error report file with more information is saved as:
Z:\ZenoShare\WindowsBuild\OpenMAMA-9b899e9\openmama_install_2.3.1\hs_err_pid2316.log

If you would like to submit a bug report, please visit:
http://bugreport.sun.com/bugreport/crash.jsp
The crash happened outside the Java Virtual Machine in native code.
See problematic frame for where to report the bug.

The issue appears to be triggered within the Qpid Proton bridge, though more
investigation will be required to determine the root cause.

Additional Information:

• The crash does not appear when processing 2 or 1 symbols.
• The crash appears regardless what shutdown interval is passed (tested with
  10, 20, and 60 second intervals).
• The crash does not seem to appear when multiple threads are used (-threads),
  though I haven't determined if there's a correlation between the number of
  threads and number of symbols which may cause issues.
• The crash is observed on both 2.3.0 and 2.3.1RC2.

Attached is a java dump file from the test run.

Replacing RB-124

When the mamac project resides in a complex path, basically consists of space
characters, the project build breaks.

Proposed solution:
In mamac project go to Property Pages -> Configuration Properties -> Build
Events -> Pre-Build Event -> Command Line should be:
"$(ProjectDir)\generateMamaVersion.bat" "$(ProjectDir)"

The solution should apply to all configuration and platforms.

As a rule of thumb on all build events in native code projects:

• All commands should be surrounded with quotes, like in
  "$(ProjectDir)\generateMamaVersion.bat"
• Only arguments that represent paths should be surrounded with quotes,
  like in "$(ProjectDir)"

xxxxBridge_getDefaultPayloadId (char **_name, char *_id)
The function retrieves the payload id which are const and hence params must be const qualified
-> (const char **_name, const char *_id).

Replacing RB-142

At present the SCons doxygen builder does not generate the MAMA docs as PDF
files automatically. We should add this ability into Scons.

There are a number of things to watch out for:

• We will need to check for the appropriate tools being available (pdflatex)
• There can be issues generating PDF's from tex files with cross-references.

It may be possible to either create a new tool/builder ourselves, or reuse an
existing builder to get this functionality. This will require some
investigation.