[PATCH] Correctly log when the queue gets dereferenced too many times


Michael Schonberg <mschonberg@...>
 

From: Mike Schonberg <mschonberg@...>

The existing logic contained a race condition which would not consistently log
the attempt to dereference a queue with a zero reference count.

The interlocked variable also requires initialization. The methods are added as
no-ops in common/wombat/wInterlocked.h for Linux, but they will have
implementations for Windows.

Signed-off-by: John Gray <jgray@...>
---
common/c_cpp/src/c/wombat/wInterlocked.h | 23 +++++++++++++++++++++++
mama/c_cpp/src/c/queue.c | 6 +++++-
mama/c_cpp/src/c/subscription.c | 3 +++
3 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/common/c_cpp/src/c/wombat/wInterlocked.h b/common/c_cpp/src/c/wombat/wInterlocked.h
index b75924e..c128804 100644
--- a/common/c_cpp/src/c/wombat/wInterlocked.h
+++ b/common/c_cpp/src/c/wombat/wInterlocked.h
@@ -69,6 +69,29 @@ adec32 (uint32_t* ptr)
typedef uint32_t wInterlockedInt;

/**
+ * This function will initialise a wInterlockedInt.
+ *
+ * @param[in] value Pointer to the item to be initialized.
+ * @return 0 on success.
+ */
+
+WCOMMONINLINE int wInterlocked_initialize(wInterlockedInt *value)
+{
+ return 0;
+}
+
+/**
+ * This function will destroy a wInterlockedInt.
+ *
+ * @param[in] value Pointer to the item to be destroyed.
+ * @return 0 on success.
+ */
+WCOMMONINLINE int wInterlocked_destroy(wInterlockedInt *value)
+{
+ return 0;
+}
+
+/**
* This function will atomically decrement a 32-bit integer value.
*
* @param[in] value Pointer to the value to be decremented.
diff --git a/mama/c_cpp/src/c/queue.c b/mama/c_cpp/src/c/queue.c
index 72e9892..7190819 100644
--- a/mama/c_cpp/src/c/queue.c
+++ b/mama/c_cpp/src/c/queue.c
@@ -198,6 +198,7 @@ mamaQueue_create (mamaQueue* queue,
impl->mQueueMonitorClosure = NULL;

/* Create the counter lock. */
+ wInterlocked_initialize(&impl->mNumberOpenObjects);
wInterlocked_set(0, &impl->mNumberOpenObjects);


@@ -436,7 +437,7 @@ mamaQueue_decrementObjectCount(mamaQueueLockHandle *handle,
int newCount = wInterlocked_decrement(&impl->mNumberOpenObjects);

/* Write a log if something has gone wrong. */
- if(impl->mNumberOpenObjects < 0)
+ if(newCount < 0)
{
mama_log(MAMA_LOG_LEVEL_ERROR, "Queue 0x%p has been dereferenced too many times.", queue);
}
@@ -719,6 +720,9 @@ mamaQueue_destroy (mamaQueue queue)
impl->mMamaQueueBridgeImpl = NULL;
impl->mMsg = NULL;

+ /* Destroy the counter lock */
+ wInterlocked_destroy(&impl->mNumberOpenObjects);
+
free (impl);

mama_log (MAMA_LOG_LEVEL_FINEST, "Leaving mamaQueue_destroy for queue 0x%X.", queue);
diff --git a/mama/c_cpp/src/c/subscription.c b/mama/c_cpp/src/c/subscription.c
index d6d0c00..6c73978 100644
--- a/mama/c_cpp/src/c/subscription.c
+++ b/mama/c_cpp/src/c/subscription.c
@@ -1831,6 +1831,9 @@ void mamaSubscriptionImpl_deallocate(mamaSubscriptionImpl *impl)
/* Destroy the mutex. */
wlock_destroy(impl->mCreateDestroyLock);

+ /* Destroy the state. */
+ wInterlocked_destroy(&impl->mState);
+
/* Free the subscription impl. */
free(impl);
}
--
1.7.5.4

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