Date   

Re: Timer race conditions

Frank Quinn <fquinn.ni@...>
 

Hi Duane,

OK I understand now, thanks - got confused between 'the thread on which timer events are firing' and the 'timer thread'.

I have given consideration in the past to simply ripping out this timer code (it's literally not used by anything other than avis / qpid bridges) and using a libevent evtimer or something like that instead directly from the bridge - I even have a mock implementation for that lying around somewhere, but that's a matter for another day.

In the interim, yeah it would be great if you could raise a bugzilla ticket with a patch for timers.c and the qpid bridge (don't worry about avis because the qpid implementation is about to replace it anyway) and we'll look to get that merged.

Cheers,
Frank

On Thu, Jul 30, 2015 at 10:01 PM, Duane Pauls <Duane.Pauls@...> wrote:

Hi Frank,

 

The mamaTimer is only being destroyed from one thread.  The problem is that the wombat timerElement is being destroyed from two threads at once, but not the mamaTimer.

 

One of the timerElement destroys is occurring from within the bridge’s timerElement callback, which is necessary to implement a reoccurring timer with a one-shot timer primitive.  The second destroy is occurring from an application call to mamaTimer_destroy().  The application has no way of knowing the timer is currently expiring.

 

The natural solution is to put locks in the bridge implementation to prevent certain things from occurring at the same time in different threads (which I believe should be done in the qpid and avis bridges), but then you end up with deadlock because the heap is holding the lock while calling the bridge’s callback.

 

In our experience with messaging API’s, you need to be very careful about holding locks while calling application callbacks.  We try to avoid this in our API’s.  I don’t see any reason the timer heap needs to hold a lock during the callback here.

 

Cheers,

Duane

 

From: Frank Quinn [mailto:fquinn.ni@...]
Sent: Thursday, July 30, 2015 3:52 PM
To: Duane Pauls
Cc: openmama-dev@...
Subject: Re: [Openmama-dev] Timer race conditions

 

Hi Duane,

 

I read this as the problem being triggered by the timer being destroyed from multiple threads is that right? If so, that is an illegal sequence of events and would constitute a bug in the application itself. From the MAMA Developer's guide section on timers:

 

"A timer can be destroyed from within a timer callback or any other callback on the queue. This function must be called from the same thread dispatching on the associated event queue unless both the default queue and dispatch queue are not actively dispatching."

 

Cheers,

Frank

 

On Thu, Jul 30, 2015 at 6:54 PM, Duane Pauls <Duane.Pauls@...> wrote:

At Solace our timer implementation in our middleware bridge is very similar to that of qpid.  So although my comments below are based primarily on our experience with our own middleware bridge, I believe the problems described apply equally to qpid.  The natural fix for the problem, as I’ll explain below, leads to the possibility of deadlock, which I believe should be fixed in common/c_cpp/src/c/timers.c.

 

I’d like to get feedback on my assessment, to see if we agree that the timers module should be modified in OpenMAMA.  Or, if anyone has a suggestion for another way to solve the problem, I’d be interested in that as well.

 

We discovered rare cases of crashes when destroying timers.  By looking at the dumps and inspecting the code, I believe the problem is that the bridgeImpl’s timerElement is not protected against multi-threaded access.  Consider:

1.       A timer expires.  In the timer thread, the bridge’s callback calls ‘BridgeMamaTimer_reset’, which destroys (i.e. frees) the timerElement.

2.       Before BridgeMamaTimer_reset recreates the timerElement, an application thread decides to destroy the timer (i.e. it is being destroyed as it is expiring).  The destroy flows through to the bridge, which is asked to destroy the timer.  The bridge then frees the timerElement.  This timerElement has already been freed in #1 above, leading to a crash.

 

The natural fix here is to protect the bridge’s timerElement with a lock.  In the BridgeMamaTimer_reset function, use the lock to make the destroy/create an atomic operation, and also protect accesses to the timerElement.

 

If this is done however, there is now a possibility for deadlock.  When a timer expires:

1.       timers.c ‘dispatchEntry’ locks the timer heap.

2.       The bridge timer callback is called.

3.       The bridge callback needs to reset the timer, which means taking the timer lock.

 

Now suppose an application (for example MamaTimerTestCPP_ForTimer) decides that when a timer expires it wants to destroy the timer (i.e. implement a one-shot timer).  The following sequence would occur:

1.       The application timer callback is called on the application’s queue dispatch thread.

2.       The application calls mamaTimer_destroy().

3.       mamaTimer_destroy() calls BridgeMamaTimer_destroy.  This involves taking the timerElement lock.

4.       BridgeMamaTimer_destroy calls destroyTimer, which involves taking the timer heap lock.

 

Because the above sequences take the same locks, but in different orders, we have the opportunity for deadlock.

 

A patch that I would propose to solve this issue is:

 

Index: timers.c

===================================================================

@@ -176,7 +176,9 @@

                 while (!timercmp(&ele->mTimeout, &now, >))

                 {

                     RB_REMOVE (orderedTimeRBTree_, &heap->mTimeTree, ele);

+                    wthread_mutex_unlock (&heap->mLock);

                     ele->mCb (ele, ele->mClosure);

+                    wthread_mutex_lock (&heap->mLock);

                     ele = RB_MIN (orderedTimeRBTree_, &heap->mTimeTree);

                     /* No timers left so break */

                     if (ele == NULL)

 

 

This means the timer heap lock is not held for the duration of an application callback.  It should always be safe to re-acquire the timer heap lock after the callback since something that could have been invalidated, such as an iterator, is not being used – the function calls RB_MIN to look at the first item on the heap.

                                                                                                                                                                                                                                                        

Does this seem like a reasonable approach to the problem I’ve described?  Or does someone have an alternate proposal for how to handle this in the bridge?  Or another solution with OpenMAMA code?

 

Should a bug be raised for this issue?

 

Note qpid is not currently susceptible to this issue, but I believe it is susceptible to the double-free described at the beginning.

 

Cheers,

Duane


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

 



Re: Timer race conditions

Duane Pauls <Duane.Pauls@...>
 

Hi Frank,

 

The mamaTimer is only being destroyed from one thread.  The problem is that the wombat timerElement is being destroyed from two threads at once, but not the mamaTimer.

 

One of the timerElement destroys is occurring from within the bridge’s timerElement callback, which is necessary to implement a reoccurring timer with a one-shot timer primitive.  The second destroy is occurring from an application call to mamaTimer_destroy().  The application has no way of knowing the timer is currently expiring.

 

The natural solution is to put locks in the bridge implementation to prevent certain things from occurring at the same time in different threads (which I believe should be done in the qpid and avis bridges), but then you end up with deadlock because the heap is holding the lock while calling the bridge’s callback.

 

In our experience with messaging API’s, you need to be very careful about holding locks while calling application callbacks.  We try to avoid this in our API’s.  I don’t see any reason the timer heap needs to hold a lock during the callback here.

 

Cheers,

Duane

 

From: Frank Quinn [mailto:fquinn.ni@...]
Sent: Thursday, July 30, 2015 3:52 PM
To: Duane Pauls
Cc: openmama-dev@...
Subject: Re: [Openmama-dev] Timer race conditions

 

Hi Duane,

 

I read this as the problem being triggered by the timer being destroyed from multiple threads is that right? If so, that is an illegal sequence of events and would constitute a bug in the application itself. From the MAMA Developer's guide section on timers:

 

"A timer can be destroyed from within a timer callback or any other callback on the queue. This function must be called from the same thread dispatching on the associated event queue unless both the default queue and dispatch queue are not actively dispatching."

 

Cheers,

Frank

 

On Thu, Jul 30, 2015 at 6:54 PM, Duane Pauls <Duane.Pauls@...> wrote:

At Solace our timer implementation in our middleware bridge is very similar to that of qpid.  So although my comments below are based primarily on our experience with our own middleware bridge, I believe the problems described apply equally to qpid.  The natural fix for the problem, as I’ll explain below, leads to the possibility of deadlock, which I believe should be fixed in common/c_cpp/src/c/timers.c.

 

I’d like to get feedback on my assessment, to see if we agree that the timers module should be modified in OpenMAMA.  Or, if anyone has a suggestion for another way to solve the problem, I’d be interested in that as well.

 

We discovered rare cases of crashes when destroying timers.  By looking at the dumps and inspecting the code, I believe the problem is that the bridgeImpl’s timerElement is not protected against multi-threaded access.  Consider:

1.       A timer expires.  In the timer thread, the bridge’s callback calls ‘BridgeMamaTimer_reset’, which destroys (i.e. frees) the timerElement.

2.       Before BridgeMamaTimer_reset recreates the timerElement, an application thread decides to destroy the timer (i.e. it is being destroyed as it is expiring).  The destroy flows through to the bridge, which is asked to destroy the timer.  The bridge then frees the timerElement.  This timerElement has already been freed in #1 above, leading to a crash.

 

The natural fix here is to protect the bridge’s timerElement with a lock.  In the BridgeMamaTimer_reset function, use the lock to make the destroy/create an atomic operation, and also protect accesses to the timerElement.

 

If this is done however, there is now a possibility for deadlock.  When a timer expires:

1.       timers.c ‘dispatchEntry’ locks the timer heap.

2.       The bridge timer callback is called.

3.       The bridge callback needs to reset the timer, which means taking the timer lock.

 

Now suppose an application (for example MamaTimerTestCPP_ForTimer) decides that when a timer expires it wants to destroy the timer (i.e. implement a one-shot timer).  The following sequence would occur:

1.       The application timer callback is called on the application’s queue dispatch thread.

2.       The application calls mamaTimer_destroy().

3.       mamaTimer_destroy() calls BridgeMamaTimer_destroy.  This involves taking the timerElement lock.

4.       BridgeMamaTimer_destroy calls destroyTimer, which involves taking the timer heap lock.

 

Because the above sequences take the same locks, but in different orders, we have the opportunity for deadlock.

 

A patch that I would propose to solve this issue is:

 

Index: timers.c

===================================================================

@@ -176,7 +176,9 @@

                 while (!timercmp(&ele->mTimeout, &now, >))

                 {

                     RB_REMOVE (orderedTimeRBTree_, &heap->mTimeTree, ele);

+                    wthread_mutex_unlock (&heap->mLock);

                     ele->mCb (ele, ele->mClosure);

+                    wthread_mutex_lock (&heap->mLock);

                     ele = RB_MIN (orderedTimeRBTree_, &heap->mTimeTree);

                     /* No timers left so break */

                     if (ele == NULL)

 

 

This means the timer heap lock is not held for the duration of an application callback.  It should always be safe to re-acquire the timer heap lock after the callback since something that could have been invalidated, such as an iterator, is not being used – the function calls RB_MIN to look at the first item on the heap.

                                                                                                                                                                                                                                                        

Does this seem like a reasonable approach to the problem I’ve described?  Or does someone have an alternate proposal for how to handle this in the bridge?  Or another solution with OpenMAMA code?

 

Should a bug be raised for this issue?

 

Note qpid is not currently susceptible to this issue, but I believe it is susceptible to the double-free described at the beginning.

 

Cheers,

Duane


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

 


Re: Timer race conditions

Frank Quinn <fquinn.ni@...>
 

Hi Duane,

I read this as the problem being triggered by the timer being destroyed from multiple threads is that right? If so, that is an illegal sequence of events and would constitute a bug in the application itself. From the MAMA Developer's guide section on timers:

"A timer can be destroyed from within a timer callback or any other callback on the queue. This function must be called from the same thread dispatching on the associated event queue unless both the default queue and dispatch queue are not actively dispatching."

Cheers,
Frank

On Thu, Jul 30, 2015 at 6:54 PM, Duane Pauls <Duane.Pauls@...> wrote:

At Solace our timer implementation in our middleware bridge is very similar to that of qpid.  So although my comments below are based primarily on our experience with our own middleware bridge, I believe the problems described apply equally to qpid.  The natural fix for the problem, as I’ll explain below, leads to the possibility of deadlock, which I believe should be fixed in common/c_cpp/src/c/timers.c.

 

I’d like to get feedback on my assessment, to see if we agree that the timers module should be modified in OpenMAMA.  Or, if anyone has a suggestion for another way to solve the problem, I’d be interested in that as well.

 

We discovered rare cases of crashes when destroying timers.  By looking at the dumps and inspecting the code, I believe the problem is that the bridgeImpl’s timerElement is not protected against multi-threaded access.  Consider:

1.       A timer expires.  In the timer thread, the bridge’s callback calls ‘BridgeMamaTimer_reset’, which destroys (i.e. frees) the timerElement.

2.       Before BridgeMamaTimer_reset recreates the timerElement, an application thread decides to destroy the timer (i.e. it is being destroyed as it is expiring).  The destroy flows through to the bridge, which is asked to destroy the timer.  The bridge then frees the timerElement.  This timerElement has already been freed in #1 above, leading to a crash.

 

The natural fix here is to protect the bridge’s timerElement with a lock.  In the BridgeMamaTimer_reset function, use the lock to make the destroy/create an atomic operation, and also protect accesses to the timerElement.

 

If this is done however, there is now a possibility for deadlock.  When a timer expires:

1.       timers.c ‘dispatchEntry’ locks the timer heap.

2.       The bridge timer callback is called.

3.       The bridge callback needs to reset the timer, which means taking the timer lock.

 

Now suppose an application (for example MamaTimerTestCPP_ForTimer) decides that when a timer expires it wants to destroy the timer (i.e. implement a one-shot timer).  The following sequence would occur:

1.       The application timer callback is called on the application’s queue dispatch thread.

2.       The application calls mamaTimer_destroy().

3.       mamaTimer_destroy() calls BridgeMamaTimer_destroy.  This involves taking the timerElement lock.

4.       BridgeMamaTimer_destroy calls destroyTimer, which involves taking the timer heap lock.

 

Because the above sequences take the same locks, but in different orders, we have the opportunity for deadlock.

 

A patch that I would propose to solve this issue is:

 

Index: timers.c

===================================================================

@@ -176,7 +176,9 @@

                 while (!timercmp(&ele->mTimeout, &now, >))

                 {

                     RB_REMOVE (orderedTimeRBTree_, &heap->mTimeTree, ele);

+                    wthread_mutex_unlock (&heap->mLock);

                     ele->mCb (ele, ele->mClosure);

+                    wthread_mutex_lock (&heap->mLock);

                     ele = RB_MIN (orderedTimeRBTree_, &heap->mTimeTree);

                     /* No timers left so break */

                     if (ele == NULL)

 

 

This means the timer heap lock is not held for the duration of an application callback.  It should always be safe to re-acquire the timer heap lock after the callback since something that could have been invalidated, such as an iterator, is not being used – the function calls RB_MIN to look at the first item on the heap.

                                                                                                                                                                                                                                                        

Does this seem like a reasonable approach to the problem I’ve described?  Or does someone have an alternate proposal for how to handle this in the bridge?  Or another solution with OpenMAMA code?

 

Should a bug be raised for this issue?

 

Note qpid is not currently susceptible to this issue, but I believe it is susceptible to the double-free described at the beginning.

 

Cheers,

Duane


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



Timer race conditions

Duane Pauls <Duane.Pauls@...>
 

At Solace our timer implementation in our middleware bridge is very similar to that of qpid.  So although my comments below are based primarily on our experience with our own middleware bridge, I believe the problems described apply equally to qpid.  The natural fix for the problem, as I’ll explain below, leads to the possibility of deadlock, which I believe should be fixed in common/c_cpp/src/c/timers.c.

 

I’d like to get feedback on my assessment, to see if we agree that the timers module should be modified in OpenMAMA.  Or, if anyone has a suggestion for another way to solve the problem, I’d be interested in that as well.

 

We discovered rare cases of crashes when destroying timers.  By looking at the dumps and inspecting the code, I believe the problem is that the bridgeImpl’s timerElement is not protected against multi-threaded access.  Consider:

1.       A timer expires.  In the timer thread, the bridge’s callback calls ‘BridgeMamaTimer_reset’, which destroys (i.e. frees) the timerElement.

2.       Before BridgeMamaTimer_reset recreates the timerElement, an application thread decides to destroy the timer (i.e. it is being destroyed as it is expiring).  The destroy flows through to the bridge, which is asked to destroy the timer.  The bridge then frees the timerElement.  This timerElement has already been freed in #1 above, leading to a crash.

 

The natural fix here is to protect the bridge’s timerElement with a lock.  In the BridgeMamaTimer_reset function, use the lock to make the destroy/create an atomic operation, and also protect accesses to the timerElement.

 

If this is done however, there is now a possibility for deadlock.  When a timer expires:

1.       timers.c ‘dispatchEntry’ locks the timer heap.

2.       The bridge timer callback is called.

3.       The bridge callback needs to reset the timer, which means taking the timer lock.

 

Now suppose an application (for example MamaTimerTestCPP_ForTimer) decides that when a timer expires it wants to destroy the timer (i.e. implement a one-shot timer).  The following sequence would occur:

1.       The application timer callback is called on the application’s queue dispatch thread.

2.       The application calls mamaTimer_destroy().

3.       mamaTimer_destroy() calls BridgeMamaTimer_destroy.  This involves taking the timerElement lock.

4.       BridgeMamaTimer_destroy calls destroyTimer, which involves taking the timer heap lock.

 

Because the above sequences take the same locks, but in different orders, we have the opportunity for deadlock.

 

A patch that I would propose to solve this issue is:

 

Index: timers.c

===================================================================

@@ -176,7 +176,9 @@

                 while (!timercmp(&ele->mTimeout, &now, >))

                 {

                     RB_REMOVE (orderedTimeRBTree_, &heap->mTimeTree, ele);

+                    wthread_mutex_unlock (&heap->mLock);

                     ele->mCb (ele, ele->mClosure);

+                    wthread_mutex_lock (&heap->mLock);

                     ele = RB_MIN (orderedTimeRBTree_, &heap->mTimeTree);

                     /* No timers left so break */

                     if (ele == NULL)

 

 

This means the timer heap lock is not held for the duration of an application callback.  It should always be safe to re-acquire the timer heap lock after the callback since something that could have been invalidated, such as an iterator, is not being used – the function calls RB_MIN to look at the first item on the heap.

                                                                                                                                                                                                                                                        

Does this seem like a reasonable approach to the problem I’ve described?  Or does someone have an alternate proposal for how to handle this in the bridge?  Or another solution with OpenMAMA code?

 

Should a bug be raised for this issue?

 

Note qpid is not currently susceptible to this issue, but I believe it is susceptible to the double-free described at the beginning.

 

Cheers,

Duane


Re: QPID Proton bridge: multiple subscribers, one publisher

Frank Quinn <fquinn.ni@...>
 

Hi Nestor,

You definitely need a broker in the middle for that sort of implementation - point to point will not support that.

You should be able to publish data to the same MAMA source (topic namespace) from multiple instances if you're using a broker and collect all the data from a single client, but you need to be careful about publishing to the same topic across multiple instances if you're using market data subscriptions.

Market data subscriptions check the MamaSenderId and MdSeqNum on every update to detect gaps, so if the client constantly receives overlapping values for these fields on the same subscription, you'll get recap storms.

Cheers,
Frank

On Wed, Jul 29, 2015 at 7:11 PM, Macrux <kmacrux@...> wrote:
Hi Frank,

Just to solve a similar question, is it possible to have the opposite behaviour in which multiple applications (separated instances) publish data into the same topic (or source), and only one subscriber receives all that data?

Thanks in advance,


On 21 July 2015 at 13:27, Frank Quinn <fquinn.ni@...> wrote:

Hi Nestor,

If you modify each new subscriber to use a different incoming URL and reply URL, you can actually do this today, though as you can imagine it becomes a bit of a brute to configure very quickly.

The better solution though is probably the currently experimental feature-qpid-broker branch that I'm hoping will make it into OpenMAMA 2.3.4. It allows for a much easier to manage pub / sub infrastructure where in terms of transport configuration, all you do is point it to the broker - there's an example mama.properties on that branch to give you an idea: http://git.openmama.org/?p=OpenMAMA.git;a=blob;f=mama/c_cpp/src/examples/mama.properties;h=86c7b1ce3933a10b0022b89837094bdb5904b974;hb=feature-qpid-broker

Cheers,
Frank


On Tue, 21 Jul 2015 16:23 Macrux <kmacrux@...> wrote:
Hi guys,

I'd like to know if there was or there will be some change to the Qpid Proton Bridge such that I can have a publish/subscribe connection in which only one advanced publisher can have multiple subscribers, because according to the wiki, "the current QPID Proton bridge is a point-to-point implementation" and I've had to run multiple publishers (always the same) each one with a different transport, and to subscribe each client with only one publisher.

Maybe, if someone knows some technique to achieve this behavior, it would be very helpful for me.

Thanks in advance.

Kind regards,


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



Re: SNAPSHOTS Messages

Frank Quinn <fquinn.ni@...>
 

Hi Eduardo,

As discussed on IRC, this query is related to what bridge components would need wired to facilitate a traditional exchange's broadcast / refresh / gap fill requirements and there are really two parts to the story for each component - what OpenMAMA does, and how it would be implemented in a bridge. So let's go through your points and try and provide a little clarity.

1. Snapshots / Intra day restart
The OpenMAMA DQ Publisher has a concept of 'initial' requests which greets new intraday clients with a full image of the instrument to which they're subscribing. To implement this usually involves a bridge implementation for 'inbox' related functions to work to facilitate request / reply.

2. Retransmission / Recap
If using market data subscriptions, the message is injected with a sequence number which OpenMAMA then checks to detect gaps. If a gap is detected at all (at a OpenMAMA layer), OpenMAMA will then request a snapshot image with what it calls a 'recap'. I know what you're thinking - this would be a nightmare if the bridge was receiving multicast data and data is coming in gappy or out of sequence, which is why I would suggest that line arbitrage and possibly minor gap filling (if you're going to support that) is done underneath the bridge layer and invisible to MAMA. That way gaps are only reported up to MAMA when they are something that would require an image refresh to recover from. Again, this would require an inbox implementation. It's worth noting that OpenMAMA itself has no concept of 'full tick by tick gap fill', and in existing implementations of MAMA, that sort of functionality is handled exclusively at the middleware layer.

Hope this helps,

Cheers,
Frank

On Wed, Jul 29, 2015 at 5:43 PM, eduardo noe rodriguez franco <enoerodriguezf@...> wrote:
Hi everyone,

Could you help me with some questions, please?

1.- If one client or broker house made the connection when the market had been working for a short while, I mean, the client made the connection lately to the open of the market. can the client ask for a SNAPSHOT or summary of the operations? I need to build one instance to attend that kind of requests.

2.- In a multicast connection, when the client ask for a re-transmission or recap, that is point to point, but how its works? I mean, what i need to do to attend that kind of request.

Thanks in advance, and I am glad to be part of your group.

Regards

Eduardo Rodriguez


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



Re: QPID Proton bridge: multiple subscribers, one publisher

macrux
 

Hi Frank,

Just to solve a similar question, is it possible to have the opposite behaviour in which multiple applications (separated instances) publish data into the same topic (or source), and only one subscriber receives all that data?

Thanks in advance,


On 21 July 2015 at 13:27, Frank Quinn <fquinn.ni@...> wrote:

Hi Nestor,

If you modify each new subscriber to use a different incoming URL and reply URL, you can actually do this today, though as you can imagine it becomes a bit of a brute to configure very quickly.

The better solution though is probably the currently experimental feature-qpid-broker branch that I'm hoping will make it into OpenMAMA 2.3.4. It allows for a much easier to manage pub / sub infrastructure where in terms of transport configuration, all you do is point it to the broker - there's an example mama.properties on that branch to give you an idea: http://git.openmama.org/?p=OpenMAMA.git;a=blob;f=mama/c_cpp/src/examples/mama.properties;h=86c7b1ce3933a10b0022b89837094bdb5904b974;hb=feature-qpid-broker

Cheers,
Frank


On Tue, 21 Jul 2015 16:23 Macrux <kmacrux@...> wrote:
Hi guys,

I'd like to know if there was or there will be some change to the Qpid Proton Bridge such that I can have a publish/subscribe connection in which only one advanced publisher can have multiple subscribers, because according to the wiki, "the current QPID Proton bridge is a point-to-point implementation" and I've had to run multiple publishers (always the same) each one with a different transport, and to subscribe each client with only one publisher.

Maybe, if someone knows some technique to achieve this behavior, it would be very helpful for me.

Thanks in advance.

Kind regards,


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


SNAPSHOTS Messages

eduardo noe rodriguez franco <enoerodriguezf@...>
 

Hi everyone,

Could you help me with some questions, please?

1.- If one client or broker house made the connection when the market had been working for a short while, I mean, the client made the connection lately to the open of the market. can the client ask for a SNAPSHOT or summary of the operations? I need to build one instance to attend that kind of requests.

2.- In a multicast connection, when the client ask for a re-transmission or recap, that is point to point, but how its works? I mean, what i need to do to attend that kind of request.

Thanks in advance, and I am glad to be part of your group.

Regards

Eduardo Rodriguez


Re: Dropping support for Qpid Proton <= 0.6

Damian Maguire <d.maguire@...>
 

No objections here, removing support for the older versions makes sense and should help reduce the overhead of supporting the middleware. I also think we should be pretty aggressive in deprecating support in the future, especially for versions which are known to have major issues.

Cheers, 

D

Damian Maguire
Senior Sales Engineer

SR.LABS Proven High Speed Electronic Trading Solutions

Adelaide Exchange | 24-26 Adelaide Street | Belfast | UK |BT2 8GD

d.maguire@...  

+44 7835 844770



From: <openmama-dev-bounces@...> on behalf of Frank Quinn
Reply-To: "fquinn.ni@..."
Date: Tuesday, July 28, 2015 at 9:45 AM
To: openmama-dev
Subject: Re: [Openmama-dev] Dropping support for Qpid Proton <= 0.6

Hi Folks,

Last call on this - if I hear no objections within a week, I'll be removing 0.5 and 0.6 support from CI and removing the backwards compatibility macros from the code.

Cheers,
Frank

On Wed, Jul 22, 2015 at 9:28 AM, Frank Quinn <fquinn.ni@...> wrote:
Hi Folks,

I am planning on dropping support for Qpid Proton 0.5 and 0.6 in the next OpenMAMA release and instead sticking to versions 0.7+. Those guys move quickly and make small changes to interfaces and header names fairly regularly so keeping up with new releases without breaking older versions is fairly tricky, and something I'd like to at least limit.

Also, they had some compiler warnings in one of their headers when our strict parsing was enabled in 0.5 that wasn't fixed until 0.7 (see https://issues.apache.org/jira/browse/PROTON-420), so dropping support for all versions prior to 0.7 will mean we can turn -Werror back on without breaking older builds.

If anyone has any objections to this, please respond.

Cheers,
Frank


Re: Dropping support for Qpid Proton <= 0.6

Frank Quinn <fquinn.ni@...>
 

Hi Folks,

Last call on this - if I hear no objections within a week, I'll be removing 0.5 and 0.6 support from CI and removing the backwards compatibility macros from the code.

Cheers,
Frank

On Wed, Jul 22, 2015 at 9:28 AM, Frank Quinn <fquinn.ni@...> wrote:
Hi Folks,

I am planning on dropping support for Qpid Proton 0.5 and 0.6 in the next OpenMAMA release and instead sticking to versions 0.7+. Those guys move quickly and make small changes to interfaces and header names fairly regularly so keeping up with new releases without breaking older versions is fairly tricky, and something I'd like to at least limit.

Also, they had some compiler warnings in one of their headers when our strict parsing was enabled in 0.5 that wasn't fixed until 0.7 (see https://issues.apache.org/jira/browse/PROTON-420), so dropping support for all versions prior to 0.7 will mean we can turn -Werror back on without breaking older builds.

If anyone has any objections to this, please respond.

Cheers,
Frank


Re: OpenMAMA to OpenMAMDA Proxy App

Frank Quinn <f.quinn@...>
 

Hi Nestor,

 

Order book instruments are typically prefixed with a lower case ‘b’, for an example, see the bookticker example in http://www.openmama.org/blog/getting-started-part-2

 

Cheers,

Frank

 

From: openmama-dev-bounces@... [mailto:openmama-dev-bounces@...] On Behalf Of Macrux
Sent: 27 July 2015 16:22
To: openmama-dev@...
Subject: [Openmama-dev] OpenMAMA to OpenMAMDA Proxy App

 

Hello everybody,

I'm trying to figure this: I want to built an OpenMAMA proxy application, similar to MamaProxy, which listen to a source for data and then publish that data in an  topic again, but the difference is that I want to publish that data as OpenMAMDA messages, I mean, as an orderbook, just as the MamdaBookPublisher example, but I haven't been able to build the orderbook from the OpenMAMA data.

Let me to explain my setup:

I run the capture replay as in the example:

capturereplayc -S TEST -m qpid -tport pub  -dictionary data/data.dict -f data/openmama_utpcasheuro_capture.5000.10.qpid.mplay -r

Then, I run an application (similar to the MamaProxy listener) to listen to:

-S TEST -s PTBIZJYE0064.EUR.ENXL -tport sub -m qpid

And I have been trying to extract the information to build the book, but I can't find the data because I don't know the fields to extract from or the way to do it, the only thing I've recognized are the two first prices, times and sizes from each side. I'm working with the Java API.

I apologize if it's a silly question, but I'm stuck on this problem.

Thanks in advance,


Néstor.


OpenMAMA to OpenMAMDA Proxy App

macrux
 

Hello everybody,

I'm trying to figure this: I want to built an OpenMAMA proxy application, similar to MamaProxy, which listen to a source for data and then publish that data in an  topic again, but the difference is that I want to publish that data as OpenMAMDA messages, I mean, as an orderbook, just as the MamdaBookPublisher example, but I haven't been able to build the orderbook from the OpenMAMA data.

Let me to explain my setup:

I run the capture replay as in the example:

capturereplayc -S TEST -m qpid -tport pub  -dictionary data/data.dict -f data/openmama_utpcasheuro_capture.5000.10.qpid.mplay -r

Then, I run an application (similar to the MamaProxy listener) to listen to:

-S TEST -s PTBIZJYE0064.EUR.ENXL -tport sub -m qpid

And I have been trying to extract the information to build the book, but I can't find the data because I don't know the fields to extract from or the way to do it, the only thing I've recognized are the two first prices, times and sizes from each side. I'm working with the Java API.

I apologize if it's a silly question, but I'm stuck on this problem.

Thanks in advance,


Néstor.


[PATCH 14/14] AVIS: Fixed corruption bug in realloc

Frank Quinn <fquinn.ni@...>
 

There was a realloc bug in AVIS when serializing / deserializing
that should be fixed with this patch. Also added a getByteBuffer
implementation.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 mama/c_cpp/src/c/payload/avismsg/avispayload.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/mama/c_cpp/src/c/payload/avismsg/avispayload.c b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
index 20063f5..82aca1a 100644
--- a/mama/c_cpp/src/c/payload/avismsg/avispayload.c
+++ b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
@@ -406,14 +406,16 @@ avismsgPayload_serialize     (const msgPayload    msg,
                 {
                     void*vp=realloc (impl->mBuffer, impl->mBufferLen+200);
                     impl->mBuffer = vp;
-                    buffPos=&impl->mBuffer;
+                    buffPos=impl->mBuffer;
                     buffPos+=currLen;
                     impl->mBufferLen+=200;
                 }
                 *(int8_t *)(buffPos) = 2;   buffPos+=1;     currLen+=1;
                 *(int16_t *)(buffPos) = len; buffPos+=2; currLen+=2;
                 memcpy (buffPos, currField->mName, len); buffPos+=len; currLen+=len;
-                *(int64_t *)(buffPos) = currField->mValue->value.int64; buffPos+=sizeof(int64_t); currLen+=sizeof(int64_t);
+                *(int64_t *)(buffPos) = (int64_t)currField->mValue->value.int64;
+                buffPos+=sizeof(int64_t);
+                currLen+=sizeof(int64_t);
                 break;
             case TYPE_REAL64:
                 len=+ strlen(currField->mName);;
@@ -421,7 +423,7 @@ avismsgPayload_serialize     (const msgPayload    msg,
                 {
                     void*vp=realloc (impl->mBuffer, impl->mBufferLen+200);
                     impl->mBuffer = vp;
-                    buffPos=&impl->mBuffer;
+                    buffPos=impl->mBuffer;
                     buffPos+=currLen;
                     impl->mBufferLen+=200;
                 }
@@ -436,11 +438,11 @@ avismsgPayload_serialize     (const msgPayload    msg,
                 {
                     void*vp=realloc (impl->mBuffer, impl->mBufferLen+200);
                     impl->mBuffer = vp;
-                    buffPos=&impl->mBuffer;
+                    buffPos=impl->mBuffer;
                     buffPos+=currLen;
                     impl->mBufferLen+=200;
                 }
-                *(int8_t *)(buffPos) = 4;   buffPos+=1;     currLen+=1;
+                *(int8_t *)(buffPos) = TYPE_STRING;   buffPos+=1;     currLen+=1;
                 *(int16_t *)(buffPos) = len; buffPos+=2; currLen+=2;
                 memcpy (buffPos, currField->mName, len); buffPos+=len; currLen+=len;
                 *(int16_t *)(buffPos) = strlen(currField->mValue->value.str); buffPos+=2; currLen+=2;
@@ -503,7 +505,7 @@ avismsgPayload_getByteBuffer     (const msgPayload    msg,
     CHECK_NULL(bufferLength);
 
     *buffer = impl->mAvisMsg;
-
+    *bufferLength = sizeof(impl->mAvisMsg);
 
     return MAMA_STATUS_OK;
 }
@@ -2034,6 +2036,7 @@ avismsgPayloadIter_get          (msgPayloadIter  iter,
         return NULL;
     }
 
+    /* If this is a special meta field, do not consider during iteration */
     if ((strcmp(SUBJECT_FIELD_NAME, avisField(field)->mName) == 0) ||
         (strcmp(INBOX_FIELD_NAME, avisField(field)->mName)== 0))
             return (avismsgPayloadIter_next(iter,field,msg));
--
2.4.3


[PATCH 13/14] UNITTEST: Remove presumptuous isTportDisconnected test

Frank Quinn <fquinn.ni@...>
 

I don't think it's reasonable to expect a unit test around OpenMAMA
to support this function call as the nature of transport connectivity
will vary across all middlewares. In the instance of Avis, it is
never really in a disconnected state and there are doubtless other
middleware implementations that are not connection oriented at all
and for which this test doesn't really make any sense, so I'm
taking it out.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 .../c/middleware/middlewareSubscriptionTests.cpp       | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/mama/c_cpp/src/gunittest/c/middleware/middlewareSubscriptionTests.cpp b/mama/c_cpp/src/gunittest/c/middleware/middlewareSubscriptionTests.cpp
index 6f19c2e..e6eccbf 100644
--- a/mama/c_cpp/src/gunittest/c/middleware/middlewareSubscriptionTests.cpp
+++ b/mama/c_cpp/src/gunittest/c/middleware/middlewareSubscriptionTests.cpp
@@ -424,24 +424,6 @@ TEST_F (MiddlewareSubscriptionTests, getPlatformErrorInvalidSubBridge)
                status);
 }
 
-TEST_F (MiddlewareSubscriptionTests, isTportDisconnected)
-{
-    int res = NULL;
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mamaSubscription_create(parent, queue, &callbacks, source, sourceName, closure));
-
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mBridge->bridgeMamaSubscriptionCreate(&subscriber, sourceName, symbol,
-                                                    tport, queue, callbacks,
-                                                    parent, closure));
-
-    res=mBridge->bridgeMamaSubscriptionIsTportDisconnected(subscriber);
-    ASSERT_TRUE(res != NULL);
-
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mBridge->bridgeMamaSubscriptionDestroy(subscriber));
-}
-
 TEST_F (MiddlewareSubscriptionTests, isTportDisconnectedInvalid)
 {
     ASSERT_EQ (MAMA_STATUS_NULL_ARG,
--
2.4.3


[PATCH 12/14] UNITTEST: Removed duplicateReplyHandle test

Frank Quinn <fquinn.ni@...>
 

This test doesn't really make sense without the use of an inbox
so it serves no useful function at the moment, therefore I'm taking
it out.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 .../src/gunittest/c/middleware/middlewareMsgTests.cpp | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/mama/c_cpp/src/gunittest/c/middleware/middlewareMsgTests.cpp b/mama/c_cpp/src/gunittest/c/middleware/middlewareMsgTests.cpp
index fc12c55..8630bb1 100644
--- a/mama/c_cpp/src/gunittest/c/middleware/middlewareMsgTests.cpp
+++ b/mama/c_cpp/src/gunittest/c/middleware/middlewareMsgTests.cpp
@@ -290,25 +290,6 @@ TEST_F (MiddlewareMsgTests, getNativeHandleInvalidResult)
                mBridge->bridgeMamaMsgGetNativeHandle(msg,NULL));
 }
 
-TEST_F (MiddlewareMsgTests, duplicateReplyHandle)
-{
-    msgBridge msg        = NULL;
-    mamaMsg   parent     = NULL;
-    void*     result     = NULL;
-    int       destroyMsg = 1;
-
-    ASSERT_EQ(MAMA_STATUS_OK,mamaMsg_create(&parent));
-
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mBridge->bridgeMamaMsgCreate(&msg,parent));
-
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mBridge->bridgeMamaMsgDuplicateReplyHandle(msg,&result));
-
-    ASSERT_EQ(MAMA_STATUS_OK,
-              mBridge->bridgeMamaMsgDestroy(msg,destroyMsg));
-}
-
 TEST_F (MiddlewareMsgTests, duplicateReplyHandleInvalidMsgBridge)
 {
     void* result = NOT_NULL;
--
2.4.3



[PATCH 11/14] AVIS: Modified iterator to align with MAMA C++ Expectations

Frank Quinn <fquinn.ni@...>
 

MAMA expects iterators to rewind when begin is called, and
remain pointing to the first entry after the *first* time
that next is called. This change aligns with that requirement.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 mama/c_cpp/src/c/payload/avismsg/avismsgimpl.h |  1 +
 mama/c_cpp/src/c/payload/avismsg/avispayload.c | 26 +++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/mama/c_cpp/src/c/payload/avismsg/avismsgimpl.h b/mama/c_cpp/src/c/payload/avismsg/avismsgimpl.h
index f83a1cb..b9b5ef2 100644
--- a/mama/c_cpp/src/c/payload/avismsg/avismsgimpl.h
+++ b/mama/c_cpp/src/c/payload/avismsg/avismsgimpl.h
@@ -55,6 +55,7 @@ typedef struct avisIterator
     AttributesIter*     mMsgIterator;
     Attributes*         mAvisMsg;
     avisFieldPayload*   mAvisField;
+    uint64_t            mIndex;
 } avisIterator;
 
 
diff --git a/mama/c_cpp/src/c/payload/avismsg/avispayload.c b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
index e935e15..20063f5 100644
--- a/mama/c_cpp/src/c/payload/avismsg/avispayload.c
+++ b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
@@ -2046,15 +2046,30 @@ avismsgPayloadIter_next          (msgPayloadIter  iter,
                                 msgFieldPayload field,
                                 msgPayload      msg)
 {
-    avisIterator* impl = (avisIterator*) iter;
-    if (!iter || !msg || !field) return NULL;
+    avisIterator*   impl = (avisIterator*) iter;
+    msgFieldPayload ret = NULL;
 
-    if (!attributes_iter_next(impl->mMsgIterator))
-        return NULL;
+    if (!iter || !msg) return NULL;
+    /* Only advance iterator if not first run */
+    if (impl->mIndex > 0)
+    {
+        impl->mIndex++;
+        if (!attributes_iter_next(impl->mMsgIterator))
+            return NULL;
+    }
 
-    return avismsgPayloadIter_get(iter, impl->mAvisField, msg);
+    ret = avismsgPayloadIter_get(iter, impl->mAvisField, msg);
+    if (NULL != ret)
+    {
+        impl->mIndex++;
+    }
+
+    return ret;
 }
 
+/* This isn't really supported on Avis. HasNext on avis is more like "i've
+ * just moved to next, do I have anything?"
+ */
 mama_bool_t
 avismsgPayloadIter_hasNext       (msgPayloadIter iter,
                                 msgPayload     msg)
@@ -2073,6 +2088,7 @@ avismsgPayloadIter_begin         (msgPayloadIter  iter,
     avisIterator* impl = (avisIterator*) iter;
     if (!impl) return NULL;
 
+    impl->mIndex = 0;
     attributes_iter_init(impl->mMsgIterator, impl->mAvisMsg);
     return avismsgPayloadIter_get(iter, impl->mAvisField, msg);
 }
--
2.4.3



[PATCH 10/14] UNITTEST: Fixed assumption of order in iteration

Frank Quinn <fquinn.ni@...>
 

The unit tests assumed that the order of entry into a message
field will be identical to the order of extraction while iterating.
This proved not to be the case with avis and possibly other payload
implementations so these changes will remove that assumption
without losing out on any other existing test strictness.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 .../src/gunittest/c/mamamsg/msgiterationtests.cpp  | 101 ++++++++++-----------
 .../gunittest/c/payload/payloadgeneraltests.cpp    |  24 +++--
 2 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/mama/c_cpp/src/gunittest/c/mamamsg/msgiterationtests.cpp b/mama/c_cpp/src/gunittest/c/mamamsg/msgiterationtests.cpp
index b8785d4..40dc60e 100644
--- a/mama/c_cpp/src/gunittest/c/mamamsg/msgiterationtests.cpp
+++ b/mama/c_cpp/src/gunittest/c/mamamsg/msgiterationtests.cpp
@@ -26,8 +26,10 @@
 #include "mama/msg.h"
 #include <gtest/gtest.h>
 #include <cstdlib>
+#include <stdlib.h>
 #include "MainUnitTestC.h"
 #include <iostream>
+#include <map>
 #include "bridge.h"
 #include "mama/types.h"
 
@@ -249,6 +251,7 @@ protected:
     mamaMsgIterator iterator;
     mamaDictionary  dict;
     mamaMsgField    field;
+    std::map<mama_fid_t, uint64_t> values;
 };
 
 MsgNewIteratorTestC::MsgNewIteratorTestC(void)
@@ -271,11 +274,14 @@ void MsgNewIteratorTestC::SetUp(void)
 
     /* add a fields to the message. */
     mamaMsg_create    (&msg);
-    mamaMsg_addU8     (msg, "u8", 101, 8);
-    mamaMsg_addString (msg, "string", 102, "This is an iteration test.");
-    mamaMsg_addU16    (msg, "u16", 103, 16);
-    mamaMsg_addU32    (msg, "u32", 104, 32);
-    mamaMsg_addU64    (msg, "u64", 105, 64);
+    for (mama_fid_t f = 101; f < 106; f++)
+    {
+        char buf[64];
+        sprintf (buf, "field_%u", f);
+        int val = rand();
+        mamaMsg_addU64 (msg, buf, f, val);
+        values.insert(std::pair<mama_fid_t, uint64_t>(f, val));
+    }
 
     /* Build the MAMA Dictionary from our test message. */
     mamaDictionary_create (&dict);
@@ -408,16 +414,15 @@ TEST_F (MsgNewIteratorTestC, IteratorAssociateNullMsg)
 TEST_F (MsgNewIteratorTestC, IteratorBegin)
 {
     mama_fid_t      fid      = 0;
-    mama_u8_t       content  = 0;
+    mama_u64_t      content  = 0;
 
     field = mamaMsgIterator_begin (iterator);
 
     mamaMsgField_getFid (field, &fid);
-    mamaMsgField_getU8  (field, &content);
+    mamaMsgField_getU64 (field, &content);
 
     /* Check the contents of the field: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values.at(fid), content);
 }
 
 /*  Description:     Check that when passed a NULL iterator, begin returns a NULL
@@ -435,45 +440,44 @@ TEST_F (MsgNewIteratorTestC, DISABLED_IteratorBeginNullIter)
 
 /*  Description:     Call begin, check that the first field is returned, then
  *                   call next, and check that the first field is returned again,
- *                   then call next again, and ensure that the second field is
- *                   returned.
- *
- *  Expected Result: First  Check: fid - 101, value - 8
- *                   Second Check: fid - 101, value - 8
- *                   Third  Check: fid - 102, value - "This is an iteration test."
+ *                   then call next again, until all fields are confirmed.
  */
 TEST_F (MsgNewIteratorTestC, IteratorBeginNext)
 {
     mama_fid_t      fid      = 0;
-    mama_u8_t       content  = 0;
-    const char*     strContent;
+    mama_u64_t      content  = 0;
 
     field = mamaMsgIterator_begin (iterator);
 
     mamaMsgField_getFid (field, &fid);
-    mamaMsgField_getU8 (field, &content);
+    mamaMsgField_getU64 (field, &content);
 
-    /* Check the contents of the field: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    /* Check the contents of the field match legal entry */
+    ASSERT_EQ(values.at(fid), content);
 
     field = mamaMsgIterator_next (iterator);
 
     mamaMsgField_getFid (field, &fid);
-    mamaMsgField_getU8 (field, &content);
 
     /* Ensure we return the first field again: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values[fid], content);
 
-    field = mamaMsgIterator_next (iterator);
+    /* Remove from reference map */
+    values.erase(fid);
 
-    mamaMsgField_getFid      (field, &fid);
-    mamaMsgField_getString   (field, &strContent);
+    /* For the 4 remaining fields, check contents */
+    for (int i = 0; i < 4; i++)
+    {
+        field = mamaMsgIterator_next (iterator);
+        mamaMsgField_getFid (field, &fid);
+        mamaMsgField_getU64 (field, &content);
+        ASSERT_EQ(values[fid], content);
+        /* Remove from reference map */
+        values.erase(fid);
+    }
 
-    /* Ensure we return the first field again: */
-    ASSERT_EQ (102, fid);
-    ASSERT_STREQ ("This is an iteration test.", strContent);
+    /* Gotta catch 'em all */
+    EXPECT_EQ(0, values.size());
 }
 
 /*  Description:     Step into the message, determine if it has a next value,
@@ -484,24 +488,21 @@ TEST_F (MsgNewIteratorTestC, IteratorBeginNext)
 TEST_F (MsgNewIteratorTestC, IteratorHasNext)
 {
     mama_fid_t      fid      = 0;
-    mama_u8_t       content  = 0;
+    mama_u64_t      content  = 0;
     mama_bool_t     hasNext  = 0;
-    const char*     strContent;
 
     field = mamaMsgIterator_begin (iterator);
 
     mamaMsgField_getFid (field, &fid);
-    mamaMsgField_getU8 (field, &content);
+    mamaMsgField_getU64 (field, &content);
 
     /* Check the contents of the field: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values[fid], content);
 
     field = mamaMsgIterator_next (iterator);
 
     /* Ensure we return the first field again: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values[fid], content);
 
     /* Check if we have a next value: */
     hasNext = mamaMsgIterator_hasNext (iterator);
@@ -510,12 +511,11 @@ TEST_F (MsgNewIteratorTestC, IteratorHasNext)
 
     /* Get the next value */
     field = mamaMsgIterator_next (iterator);
-    mamaMsgField_getFid      (field, &fid);
-    mamaMsgField_getString   (field, &strContent);
+    mamaMsgField_getFid (field, &fid);
+    mamaMsgField_getU64 (field, &content);
 
     /* Ensure we return the first field again: */
-    ASSERT_EQ (102, fid);
-    ASSERT_STREQ ("This is an iteration test.", strContent);
+    ASSERT_EQ(values[fid], content);
 }
 
 /*  Description:     Step to the end of the message, check if it has a next,
@@ -526,23 +526,20 @@ TEST_F (MsgNewIteratorTestC, IteratorHasNext)
 TEST_F (MsgNewIteratorTestC, IteratorHasNoNext)
 {
     mama_fid_t      fid      = 0;
-    mama_u8_t       content  = 0;
-    mama_bool_t     hasNext  = 0;
+    mama_u64_t      content  = 0;
 
     field = mamaMsgIterator_begin (iterator);
 
     mamaMsgField_getFid (field, &fid);
-    mamaMsgField_getU8 (field, &content);
+    mamaMsgField_getU64 (field, &content);
 
     /* Check the contents of the field: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values[fid], content);
 
     field = mamaMsgIterator_next (iterator);
 
     /* Ensure we return the first field again: */
-    ASSERT_EQ (101, fid);
-    ASSERT_EQ (8, content);
+    ASSERT_EQ(values[fid], content);
 
     /* Move to the last message: */
     field = mamaMsgIterator_next (iterator);
@@ -550,10 +547,12 @@ TEST_F (MsgNewIteratorTestC, IteratorHasNoNext)
     field = mamaMsgIterator_next (iterator);
     field = mamaMsgIterator_next (iterator);
 
-    /* Check if we have a next value: */
-    hasNext = mamaMsgIterator_hasNext (iterator);
+    /* Move past last message - should not crash */
+    field = mamaMsgIterator_next (iterator);
+    ASSERT_EQ(NULL, field);
 
-    ASSERT_EQ (0, hasNext);
+    /* Check if we have a next value: */
+    ASSERT_EQ (0, mamaMsgIterator_hasNext (iterator));
 }
 
 /*  Description:     Attempt to check hasNext for a NULL iterator.
diff --git a/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp b/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
index 199e2ad..7b9eca8 100644
--- a/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
+++ b/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
@@ -1891,28 +1891,38 @@ TEST_F(PayloadGeneralTests, IterAssociateValid)
 
 TEST_F(PayloadGeneralTests, IterAssociateTwiceValid)
 {
-    msgPayload          testPayload = NULL;
-    msgPayload          testPayload2 = NULL;
-    msgPayloadIter      testIter = NULL;
-    msgFieldPayload     testField = NULL;
-    mama_fid_t          test_fid = 0;
+    msgPayload           testPayload = NULL;
+    msgPayload           testPayload2 = NULL;
+    msgPayloadIter       testIter = NULL;
+    msgFieldPayload      testField = NULL;
+    mama_fid_t           test_fid = 0;
+    std::set<mama_fid_t> pl1_fids;
+    std::set<mama_fid_t> pl2_fids;
 
     result = aBridge->msgPayloadCreate(&testPayload);
     EXPECT_EQ (MAMA_STATUS_OK, result);
 
     // Create 2 payloads
     aBridge->msgPayloadAddString (testPayload, "name2", 102, "Unit");
+    pl1_fids.insert(102);
     aBridge->msgPayloadAddString (testPayload, "name3", 103, "Testing");
+    pl1_fids.insert(103);
     aBridge->msgPayloadAddString (testPayload, "name4", 104, "Is");
+    pl1_fids.insert(104);
     aBridge->msgPayloadAddString (testPayload, "name5", 105, "Fun");
+    pl1_fids.insert(105);
 
     result = aBridge->msgPayloadCreate(&testPayload2);
     EXPECT_EQ (MAMA_STATUS_OK, result);
 
     aBridge->msgPayloadAddString (testPayload2, "name2", 202, "Repeating");
+    pl2_fids.insert(202);
     aBridge->msgPayloadAddString (testPayload2, "name3", 203, "Things");
+    pl2_fids.insert(203);
     aBridge->msgPayloadAddString (testPayload2, "name4", 204, "Is");
+    pl2_fids.insert(204);
     aBridge->msgPayloadAddString (testPayload2, "name5", 205, "Great");
+    pl2_fids.insert(205);
 
     // Create iterator
     result = aBridge->msgPayloadIterCreate(&testIter, testPayload);
@@ -1931,7 +1941,7 @@ TEST_F(PayloadGeneralTests, IterAssociateTwiceValid)
     testField = aBridge->msgPayloadIterNext(testIter,testField,testPayload);
 
     result = aBridge->msgFieldPayloadGetFid(testField,NULL,NULL, &test_fid);
-    EXPECT_EQ (103, test_fid);
+    EXPECT_NE (pl1_fids.find(test_fid), pl1_fids.end());
 
     // reuse iterator with new payload
     result = aBridge->msgPayloadIterAssociate(testIter, testPayload2);
@@ -1946,7 +1956,7 @@ TEST_F(PayloadGeneralTests, IterAssociateTwiceValid)
     testField = aBridge->msgPayloadIterNext(testIter,testField,testPayload2);
 
     result = aBridge->msgFieldPayloadGetFid(testField,NULL,NULL, &test_fid);
-    EXPECT_EQ (203, test_fid);
+    EXPECT_NE (pl2_fids.find(test_fid), pl2_fids.end());
 
     result = aBridge->msgPayloadIterDestroy(testIter);
     EXPECT_EQ (MAMA_STATUS_OK, result);
--
2.4.3


[PATCH 09/14] UNITTEST: Fixed msgGetDateTimeMSecInValidName to behave correctly

Frank Quinn <fquinn.ni@...>
 

This unit test was testing for the wrong thing. Really all it
should have been testing was that the methods work OK when a NULL
name is provided, but a fid is non-NULL.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp b/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
index 408dc02..cc8ff50 100644
--- a/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
+++ b/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
@@ -697,8 +697,8 @@ TEST_F (MsgGeneralTestsC, DISABLED_msgGetDateTimeMSecInValidMsg)
 
 TEST_F (MsgGeneralTestsC, msgGetDateTimeMSecInValidName)
 {
-    mama_fid_t           fid          = 0;
-    mama_u64_t*          milliseconds = 0;
+    mama_fid_t           fid          = 102;
+    mama_u64_t           milliseconds = 0;
     mamaDateTime         dateTime     = NULL;
     mamaDateTime         m_out        = NULL;
 
@@ -707,10 +707,11 @@ TEST_F (MsgGeneralTestsC, msgGetDateTimeMSecInValidName)
     mamaDateTime_create(&m_out);
     mamaDateTime_setToNow(dateTime);
    
-    mamaMsg_addDateTime(mMsg, NULL, 102, dateTime);
-    mamaMsg_getDateTime(mMsg, NULL, 102, m_out);
+    mamaMsg_addDateTime(mMsg, NULL, fid, dateTime);
+    mamaMsg_getDateTime(mMsg, NULL, fid, m_out);
 
-    ASSERT_EQ (mamaMsg_getDateTimeMSec(mMsg, NULL, fid, milliseconds), MAMA_STATUS_INVALID_ARG);
+    /* Invalid name is ok, as long as fid is specified */
+    ASSERT_EQ (MAMA_STATUS_OK, mamaMsg_getDateTimeMSec(mMsg, NULL, fid, &milliseconds));
 }
 
 TEST_F (MsgGeneralTestsC, msgGetDateTimeMSecInValidFid)
--
2.4.3


[PATCH 08/14] AVIS: Added opaque data type serialization and deserialization

Frank Quinn <fquinn.ni@...>
 

There was already support for opaque data types but they could
not be serialized or deserialized. This patch should remedy
this.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 mama/c_cpp/src/c/payload/avismsg/avispayload.c | 53 ++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/mama/c_cpp/src/c/payload/avismsg/avispayload.c b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
index 0089fdc..e935e15 100644
--- a/mama/c_cpp/src/c/payload/avismsg/avispayload.c
+++ b/mama/c_cpp/src/c/payload/avismsg/avispayload.c
@@ -306,7 +306,27 @@ avismsgPayload_unSerialize (const msgPayload    msg,
                 avisMsg_setString(impl->mAvisMsg, tempName, 0, impl->mStringBuffer);
                 break;
             case TYPE_OPAQUE:
+            {
+                uint64_t dataSize = 0;
+                /* Pull out the field id length */
+                len=*(int16_t *)(buffPos);
+                buffPos+=sizeof(int16_t);
+                currLen+=sizeof(int16_t);
+                /* Pull out the field id and NULL terminate */
+                memcpy (tempName, buffPos, len);
+                buffPos+=len;
+                currLen+=len;
+                tempName[len]='\0';
+                /* Pull out number of bytes in opaque data */
+                dataSize=*(uint64_t*)(buffPos);
+                buffPos+=sizeof(uint64_t);
+                currLen+=sizeof(uint64_t);
+                /* Use raw data directly */
+                avisMsg_setOpaque(impl->mAvisMsg, tempName, 0, buffPos, dataSize);
+                buffPos+=dataSize;
+                currLen+=dataSize;
                 break;
+            }
         }
     }
 
@@ -428,6 +448,39 @@ avismsgPayload_serialize     (const msgPayload    msg,
                 buffPos+=strlen(currField->mValue->value.str); currLen+=strlen(currField->mValue->value.str);
                 break;
             case TYPE_OPAQUE:
+                len=+ strlen(currField->mName);
+                /* Current length + 1 for type, 2 for name size + length of name
+                 * + 8 bytes for data size + data size */
+                if (impl->mBufferLen < currLen+3+len+8+currField->mValue->value.bytes.item_count)
+                {
+                    void*vp=realloc (impl->mBuffer, impl->mBufferLen+200);
+                    impl->mBuffer = vp;
+                    buffPos=impl->mBuffer;
+                    buffPos+=currLen;
+                    impl->mBufferLen+=200;
+                }
+                /* Copy data type */
+                *(int8_t *)(buffPos) = 5;
+                buffPos+=1;
+                currLen+=1;
+                /* Copy field id length */
+                *(int16_t *)(buffPos) = len;
+                buffPos+=2;
+                currLen+=2;
+                /* Copy field id itself */
+                memcpy (buffPos, currField->mName, len);
+                buffPos+=len;
+                currLen+=len;
+                /* Copy number of bytes in data */
+                *(uint64_t *)(buffPos) = (uint64_t)currField->mValue->value.bytes.item_count;
+                buffPos+=sizeof(uint64_t);
+                currLen+=sizeof(uint64_t);
+                /* Copy the opaque data itself */
+                memcpy (buffPos,
+                        currField->mValue->value.bytes.items,
+                        currField->mValue->value.bytes.item_count);
+                buffPos+=currField->mValue->value.bytes.item_count;
+                currLen+=currField->mValue->value.bytes.item_count;
                 break;
 
         }
--
2.4.3


[PATCH 07/14] UNITTEST: Added some working getSendSubject unit tests

Frank Quinn <fquinn.ni@...>
 

The previous tests pretty much assumed it wasn't implemented in the
bridge. This fix adds some basic testing around this functionality
for self-describing messages.

Signed-off-by: Frank Quinn <fquinn.ni@...>
---
 .../src/gunittest/c/mamamsg/msggeneraltests.cpp    | 10 ++++++---
 .../gunittest/c/payload/payloadgeneraltests.cpp    | 25 ++++++++++++++++++----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp b/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
index a7bc7a9..408dc02 100644
--- a/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
+++ b/mama/c_cpp/src/gunittest/c/mamamsg/msggeneraltests.cpp
@@ -531,18 +531,22 @@ TEST_F (MsgGeneralTestsC, DISABLED_msgImplSetDqStrategyContextInValidDq)
 */
 TEST_F (MsgGeneralTestsC, msgGetSendSubjectValid)
 {
-    const char*       subject    = "";
+    const char*       subjectIn  = "subject";
+    const char*       subjectOut = NULL;
     mama_status       status;
   
     //add fields to msg
     mamaMsg_addString (mMsg, "name0", 101, "test0");
     mamaMsg_addString (mMsg, "name1", 102, "test1");
-   
-    status = mamaMsg_getSendSubject(mMsg, &subject);
+
+    mamaMsgImpl_setBridgeImpl (mMsg, mMiddlewareBridge);
+    mamaMsgImpl_setSubscInfo (mMsg, NULL, NULL, subjectIn, 1);
+    status = mamaMsg_getSendSubject(mMsg, &subjectOut);
 
     CHECK_NON_IMPLEMENTED_OPTIONAL(status);
 
     ASSERT_EQ (status, MAMA_STATUS_OK);
+    EXPECT_STREQ(subjectIn, subjectOut);
 }
 
 TEST_F (MsgGeneralTestsC, msgGetSendSubjectInValidMsg)
diff --git a/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp b/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
index bf67cb0..199e2ad 100644
--- a/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
+++ b/mama/c_cpp/src/gunittest/c/payload/payloadgeneraltests.cpp
@@ -40,11 +40,13 @@ protected:
     virtual void TearDown(void);
 
     mamaPayloadBridge   aBridge;
+    mamaBridge          mMiddlewareBridge;
     mama_status         result;
 };
 
 PayloadGeneralTests::PayloadGeneralTests(void)
     : aBridge (NULL)
+    , mMiddlewareBridge (NULL)
     , result (MAMA_STATUS_OK)
 {
 }
@@ -56,6 +58,7 @@ PayloadGeneralTests::~PayloadGeneralTests(void)
 void PayloadGeneralTests::SetUp(void)
 {
     mama_loadPayloadBridge (&aBridge,getPayload());
+    mama_loadBridge (&mMiddlewareBridge, getMiddleware());
 }
 
 void PayloadGeneralTests::TearDown(void)
@@ -534,12 +537,23 @@ TEST_F(PayloadGeneralTests, GetNumberFieldsInValidNumFields)
 TEST_F(PayloadGeneralTests, GetSendSubjectValid)
 {
     msgPayload          testPayload = NULL;
+    const char*         subjectOut  = NULL;
+    const char*         subjectIn   = "testsubj";
+    msgBridge           bridgeMsg   = NULL;
+    mamaMsg             parentMsg   = NULL;
 
-    result = aBridge->msgPayloadCreate(&testPayload);
+    //result = aBridge->msgPayloadCreate(&testPayload);
+    mamaMsg_createForPayloadBridge(&parentMsg, aBridge);
+    mamaMsgImpl_getPayload(parentMsg, &testPayload);
     EXPECT_EQ (MAMA_STATUS_OK, result);
 
-    result = aBridge->msgPayloadGetSendSubject(testPayload, NULL);
-    EXPECT_EQ (MAMA_STATUS_NOT_IMPLEMENTED, result);
+    mamaMsgImpl_setBridgeImpl (parentMsg, mMiddlewareBridge);
+    mamaMsgImpl_setSubscInfo (parentMsg, NULL, NULL, subjectIn, 1);
+
+    result = aBridge->msgPayloadGetSendSubject(testPayload, &subjectOut);
+    CHECK_NON_IMPLEMENTED_OPTIONAL(result);
+
+    EXPECT_EQ (MAMA_STATUS_OK, result);
 }
 
 
@@ -562,7 +576,10 @@ TEST_F(PayloadGeneralTests, GetSendSubjectInValidSubject)
     EXPECT_EQ (MAMA_STATUS_OK, result);
 
     result = aBridge->msgPayloadGetSendSubject(testPayload, NULL);
-    EXPECT_EQ (MAMA_STATUS_NOT_IMPLEMENTED, result);
+
+    CHECK_NON_IMPLEMENTED_OPTIONAL(result);
+
+    EXPECT_EQ (MAMA_STATUS_NULL_ARG, result);
 }
 
 TEST_F(PayloadGeneralTests, ToStringValid)
--
2.4.3

741 - 760 of 2312