From 46aec931d02cce3af99102819cb42b984e2c02ea Mon Sep 17 00:00:00 2001 From: Ian Craggs Date: Wed, 13 Nov 2013 11:33:41 +0000 Subject: [PATCH] Changes to the synchronous client API to check mutex calls on Linux, for bug #419233 --- Makefile | 51 ++++++++++++++++++++++++++++++++++++------------ build.xml | 42 +++++++++++++++++++-------------------- src/MQTTAsync.c | 39 ++++-------------------------------- src/MQTTClient.c | 12 ++++++++++++ 4 files changed, 75 insertions(+), 69 deletions(-) diff --git a/Makefile b/Makefile index 4206ddb8..4a21e435 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,18 @@ SYNC_SAMPLES = ${addprefix ${blddir}/samples/,${SAMPLE_FILES_C}} SAMPLE_FILES_A = stdoutsuba MQTTAsync_subscribe MQTTAsync_publish ASYNC_SAMPLES = ${addprefix ${blddir}/samples/,${SAMPLE_FILES_A}} +TEST_FILES_C = test1 +SYNC_TESTS = ${addprefix ${blddir}/test/,${TEST_FILES_C}} + +TEST_FILES_CS = test3 +SYNC_SSL_TESTS = ${addprefix ${blddir}/test/,${TEST_FILES_CS}} + +TEST_FILES_A = test4 +ASYNC_TESTS = ${addprefix ${blddir}/test/,${TEST_FILES_A}} + +TEST_FILES_AS = test5 +ASYNC_SSL_TESTS = ${addprefix ${blddir}/test/,${TEST_FILES_AS}} + # The names of the four different libraries to be built MQTTLIB_C = paho-mqtt3c MQTTLIB_CS = paho-mqtt3cs @@ -96,20 +108,33 @@ MQTTVERSION_TARGET = ${blddir}/MQTTVersion CCFLAGS_SO = -g -fPIC -Os -Wall -fvisibility=hidden FLAGS_EXE = -I ${srcdir} -lpthread -L ${blddir} -LDFLAGS_C = -shared -Wl,-soname,lib$(MQTTLIB_C).so.${MAJOR_VERSION} -LDFLAGS_CS = -shared -Wl,-soname,lib$(MQTTLIB_CS).so.${MAJOR_VERSION} -ldl -Wl,-whole-archive -lcrypto -lssl -Wl,-no-whole-archive -LDFLAGS_A = -shared -Wl,-soname,lib${MQTTLIB_A}.so.${MAJOR_VERSION} -LDFLAGS_AS = -shared -Wl,-soname,lib${MQTTLIB_AS}.so.${MAJOR_VERSION} -ldl -Wl,-whole-archive -lcrypto -lssl -Wl,-no-whole-archive +LDFLAGS_C = -shared -Wl,-soname,lib$(MQTTLIB_C).so.${MAJOR_VERSION} -Wl,-init,MQTTClient_init +LDFLAGS_CS = -shared -Wl,-soname,lib$(MQTTLIB_CS).so.${MAJOR_VERSION} -ldl -lcrypto -lssl -Wl,-no-whole-archive -Wl,-init,MQTTClient_init +LDFLAGS_A = -shared -Wl,-soname,lib${MQTTLIB_A}.so.${MAJOR_VERSION} -Wl,-init,MQTTAsync_init +LDFLAGS_AS = -shared -Wl,-soname,lib${MQTTLIB_AS}.so.${MAJOR_VERSION} -ldl -lcrypto -lssl -Wl,-no-whole-archive -Wl,-init,MQTTAsync_init all: build -build: | mkdir ${MQTTLIB_C_TARGET} ${MQTTLIB_CS_TARGET} ${MQTTLIB_A_TARGET} ${MQTTLIB_AS_TARGET} ${MQTTVERSION_TARGET} ${SYNC_SAMPLES} ${ASYNC_SAMPLES} +build: | mkdir ${MQTTLIB_C_TARGET} ${MQTTLIB_CS_TARGET} ${MQTTLIB_A_TARGET} ${MQTTLIB_AS_TARGET} ${MQTTVERSION_TARGET} ${SYNC_SAMPLES} ${ASYNC_SAMPLES} ${SYNC_TESTS} ${SYNC_SSL_TESTS} ${ASYNC_TESTS} ${ASYNC_SSL_TESTS} clean: rm -rf ${blddir}/* mkdir: -mkdir -p ${blddir}/samples + -mkdir -p ${blddir}/test + +${SYNC_TESTS}: ${blddir}/test/%: ${srcdir}/../test/%.c + ${CC} ${FLAGS_EXE} -g -o ${blddir}/test/${basename ${+F}} $< -l${MQTTLIB_C} + +${SYNC_SSL_TESTS}: ${blddir}/test/%: ${srcdir}/../test/%.c + ${CC} ${FLAGS_EXE} -g -o ${blddir}/test/${basename ${+F}} $< -l${MQTTLIB_CS} + +${ASYNC_TESTS}: ${blddir}/test/%: ${srcdir}/../test/%.c + ${CC} ${FLAGS_EXE} -g -o ${blddir}/test/${basename ${+F}} $< -l${MQTTLIB_A} + +${ASYNC_SSL_TESTS}: ${blddir}/test/%: ${srcdir}/../test/%.c + ${CC} ${FLAGS_EXE} -g -o ${blddir}/test/${basename ${+F}} $< -l${MQTTLIB_AS} ${SYNC_SAMPLES}: ${blddir}/samples/%: ${srcdir}/samples/%.c ${CC} ${FLAGS_EXE} -o ${blddir}/samples/${basename ${+F}} $< -l${MQTTLIB_C} @@ -119,23 +144,23 @@ ${ASYNC_SAMPLES}: ${blddir}/samples/%: ${srcdir}/samples/%.c ${MQTTLIB_C_TARGET}: ${SOURCE_FILES_C} ${HEADERS_C} ${CC} ${CCFLAGS_SO} ${LDFLAGS_C} -o $@ ${SOURCE_FILES_C} - ln -s lib$(MQTTLIB_C).so.${VERSION} ${blddir}/lib$(MQTTLIB_C).so.${MAJOR_VERSION} - ln -s lib$(MQTTLIB_C).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_C).so + -ln -s lib$(MQTTLIB_C).so.${VERSION} ${blddir}/lib$(MQTTLIB_C).so.${MAJOR_VERSION} + -ln -s lib$(MQTTLIB_C).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_C).so ${MQTTLIB_CS_TARGET}: ${SOURCE_FILES_CS} ${HEADERS_C} ${CC} ${CCFLAGS_SO} ${LDFLAGS_CS} -o $@ ${SOURCE_FILES_CS} -DOPENSSL - ln -s lib$(MQTTLIB_CS).so.${VERSION} ${blddir}/lib$(MQTTLIB_CS).so.${MAJOR_VERSION} - ln -s lib$(MQTTLIB_CS).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_CS).so + -ln -s lib$(MQTTLIB_CS).so.${VERSION} ${blddir}/lib$(MQTTLIB_CS).so.${MAJOR_VERSION} + -ln -s lib$(MQTTLIB_CS).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_CS).so ${MQTTLIB_A_TARGET}: ${SOURCE_FILES_A} ${HEADERS_A} ${CC} ${CCFLAGS_SO} ${LDFLAGS_A} -o $@ ${SOURCE_FILES_A} - ln -s lib$(MQTTLIB_A).so.${VERSION} ${blddir}/lib$(MQTTLIB_A).so.${MAJOR_VERSION} - ln -s lib$(MQTTLIB_A).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_A).so + -ln -s lib$(MQTTLIB_A).so.${VERSION} ${blddir}/lib$(MQTTLIB_A).so.${MAJOR_VERSION} + -ln -s lib$(MQTTLIB_A).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_A).so ${MQTTLIB_AS_TARGET}: ${SOURCE_FILES_AS} ${HEADERS_A} ${CC} ${CCFLAGS_SO} ${LDFLAGS_AS} -o $@ ${SOURCE_FILES_AS} -DOPENSSL - ln -s lib$(MQTTLIB_AS).so.${VERSION} ${blddir}/lib$(MQTTLIB_AS).so.${MAJOR_VERSION} - ln -s lib$(MQTTLIB_AS).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_AS).so + -ln -s lib$(MQTTLIB_AS).so.${VERSION} ${blddir}/lib$(MQTTLIB_AS).so.${MAJOR_VERSION} + -ln -s lib$(MQTTLIB_AS).so.${MAJOR_VERSION} ${blddir}/lib$(MQTTLIB_AS).so ${MQTTVERSION_TARGET}: $(srcdir)/MQTTVersion.c $(srcdir)/MQTTAsync.h ${CC} ${FLAGS_EXE} -o $@ -l${MQTTLIB_A} $(srcdir)/MQTTVersion.c -ldl diff --git a/build.xml b/build.xml index adf6dd83..fbaabe84 100644 --- a/build.xml +++ b/build.xml @@ -57,7 +57,7 @@ - + @@ -69,7 +69,7 @@ - + @@ -210,22 +210,22 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + - + @@ -234,13 +234,13 @@ - - - + + + - + - + diff --git a/src/MQTTAsync.c b/src/MQTTAsync.c index e1144c4a..f3b2051d 100644 --- a/src/MQTTAsync.c +++ b/src/MQTTAsync.c @@ -91,10 +91,10 @@ BOOL APIENTRY DllMain(HANDLE hModule, mqttasync_mutex = CreateMutex(NULL, 0, NULL); mqttcommand_mutex = CreateMutex(NULL, 0, NULL); send_sem = CreateEvent( - NULL, // default security attributes - FALSE, // manual-reset event? - FALSE, // initial state is nonsignaled - NULL // object name + NULL, /* default security attributes */ + FALSE, /* manual-reset event? */ + FALSE, /* initial state is nonsignaled */ + NULL /* object name */ ); stack_mutex = CreateMutex(NULL, 0, NULL); heap_mutex = CreateMutex(NULL, 0, NULL); @@ -753,16 +753,12 @@ void MQTTAsync_checkDisconnect(MQTTAsync handle, MQTTAsync_command* command) if (command->details.dis.internal && m->cl && was_connected) { Log(TRACE_MIN, -1, "Calling connectionLost for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->cl))(m->context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } else if (!command->details.dis.internal && command->onSuccess) { Log(TRACE_MIN, -1, "Calling disconnect complete for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->onSuccess))(command->context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } FUNC_EXIT; @@ -1051,9 +1047,7 @@ void MQTTAsync_processCommand() data.alt.pub.message.qos = command->command.details.pub.qos; data.alt.pub.message.retained = command->command.details.pub.retained; Log(TRACE_MIN, -1, "Calling publish success for client %s", command->client->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->command.onSuccess))(command->command.context, &data); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } else @@ -1115,10 +1109,7 @@ void MQTTAsync_processCommand() if (command->command.onFailure) { Log(TRACE_MIN, -1, "Calling command failure for client %s", command->client->c->clientID); - - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->command.onFailure))(command->command.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } MQTTAsync_freeConnect(command->command); MQTTAsync_freeCommand(command); /* free up the command if necessary */ @@ -1182,9 +1173,7 @@ void MQTTAsync_checkTimeouts() if (m->connect.onFailure) { Log(TRACE_MIN, -1, "Calling connect failure for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->connect.onFailure))(m->connect.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } continue; @@ -1208,9 +1197,7 @@ void MQTTAsync_checkTimeouts() { Log(TRACE_MIN, -1, "Calling %s failure for client %s", MQTTPacket_name(com->command.type), m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(com->command.onFailure))(com->command.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } timed_out_count++; } @@ -1494,9 +1481,7 @@ thread_return_type WINAPI MQTTAsync_receiveThread(void* n) if (m->connect.onSuccess) { Log(TRACE_MIN, -1, "Calling connect success for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->connect.onSuccess))(m->connect.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } else @@ -1527,9 +1512,7 @@ thread_return_type WINAPI MQTTAsync_receiveThread(void* n) data.code = rc; data.message = "CONNACK return code"; Log(TRACE_MIN, -1, "Calling connect failure for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->connect.onFailure))(m->connect.context, &data); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } } @@ -1567,9 +1550,7 @@ thread_return_type WINAPI MQTTAsync_receiveThread(void* n) rc = MQTTProtocol_handleSubacks(pack, m->c->net.socket); handleCalled = 1; Log(TRACE_MIN, -1, "Calling subscribe success for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->command.onSuccess))(command->command.context, &data); - //MQTTAsync_lock_mutex(mqttasync_mutex); if (array) free(array); } @@ -1598,9 +1579,7 @@ thread_return_type WINAPI MQTTAsync_receiveThread(void* n) rc = MQTTProtocol_handleUnsubacks(pack, m->c->net.socket); handleCalled = 1; Log(TRACE_MIN, -1, "Calling unsubscribe success for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->command.onSuccess))(command->command.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } MQTTAsync_freeCommand(command); break; @@ -1932,9 +1911,7 @@ int MQTTAsync_deliverMessage(MQTTAsyncs* m, char* topicName, int topicLen, MQTTA Log(TRACE_MIN, -1, "Calling messageArrived for client %s, queue depth %d", m->c->clientID, m->c->messageQueue->count); - //MQTTAsync_unlock_mutex(mqttasync_mutex); rc = (*(m->ma))(m->context, topicName, topicLen, mm); - //MQTTAsync_lock_mutex(mqttasync_mutex); /* if 0 (false) is returned by the callback then it failed, so we don't remove the message from * the queue, and it will be retried later. If 1 is returned then the message data may have been freed, * so we must be careful how we use it. @@ -2594,9 +2571,7 @@ exit: if (m->connect.onFailure) { Log(TRACE_MIN, -1, "Calling connect failure for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->connect.onFailure))(m->connect.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } } @@ -2675,9 +2650,7 @@ MQTTPacket* MQTTAsync_cycle(int* sock, unsigned long timeout, int* rc) if (m->connect.onFailure) { Log(TRACE_MIN, -1, "Calling connect failure for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->connect.onFailure))(m->connect.context, NULL); - //MQTTAsync_lock_mutex(mqttasync_mutex); } } } @@ -2706,9 +2679,7 @@ MQTTPacket* MQTTAsync_cycle(int* sock, unsigned long timeout, int* rc) if (m->dc) { Log(TRACE_MIN, -1, "Calling deliveryComplete for client %s, msgid %d", m->c->clientID, msgid); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(m->dc))(m->context, msgid); - //MQTTAsync_lock_mutex(mqttasync_mutex); } /* use the msgid to find the callback to be called */ while (ListNextElement(m->responses, ¤t)) @@ -2729,9 +2700,7 @@ MQTTPacket* MQTTAsync_cycle(int* sock, unsigned long timeout, int* rc) data.alt.pub.message.qos = command->command.details.pub.qos; data.alt.pub.message.retained = command->command.details.pub.retained; Log(TRACE_MIN, -1, "Calling publish success for client %s", m->c->clientID); - //MQTTAsync_unlock_mutex(mqttasync_mutex); (*(command->command.onSuccess))(command->command.context, &data); - //MQTTAsync_lock_mutex(mqttasync_mutex); } MQTTAsync_freeCommand(command); break; diff --git a/src/MQTTClient.c b/src/MQTTClient.c index 1e2f1e40..2e6384ab 100644 --- a/src/MQTTClient.c +++ b/src/MQTTClient.c @@ -97,6 +97,18 @@ BOOL APIENTRY DllMain(HANDLE hModule, #else static pthread_mutex_t mqttclient_mutex_store = PTHREAD_MUTEX_INITIALIZER; static mutex_type mqttclient_mutex = &mqttclient_mutex_store; + +void MQTTClient_init() +{ + pthread_mutexattr_t attr; + int rc; + + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_ERRORCHECK); + if ((rc = pthread_mutex_init(mqttclient_mutex, &attr)) != 0) + printf("MQTTAsync: error %d initializing client_mutex\n", rc); +} + #define WINAPI #endif