search

LiamBindle / MQTT-C

A portable MQTT C client for embedded systems and PCs alike.
https://liambindle.ca/MQTT-C
MIT License
768 stars 270 forks source link

Set MQTT_PAL_MUTEX_INIT to use a recursive mutex #128

Closed Monarda closed 3 years ago

Monarda commented 3 years ago

This is pretty clearly an alternate take on @basilaljamal 's pull request #123, and an attempt at a solution to issue #88.

I've moved all the code changes into mqtt_pal.h and made it platform-specific to POSIX platforms, by redefining MQTT_PAL_MUTEX_INIT. But I'm not sure if this is the right approach. Should MQTT_PAL_MUTEX_INIT be converted into a function instead of a multiline define? Should a define switch be used to control whether recursive mutexes are used?

#ifndef MQTT_USE_MUTEX_RECURSIVE
#define MQTT_PAL_MUTEX_INIT(mtx_ptr) pthread_mutex_init(mtx_ptr, NULL)
#else
#define MQTT_PAL_MUTEX_INIT(mtx_ptr) { ... }
#endif

Happy for feedback!

yamt commented 3 years ago

does this mean to change the semantics of the pal api?

LiamBindle commented 3 years ago

@yamt This shouldn't change the semantics of the PAL. It would prevent a deadlock in the default Unix/APPLE PALs if one tries to call mqtt_publish() in their receive callback.

That said, I think the receive callbacks should be thought of as interrupts (fast call that essentially mark a received message) rather than heavy-duty functions, so I'm a bit hesistant to "fix" this.

I'm going to close this due to inactivity.