Mosquitto library should default to thread-safe

[AndrewCarterUK]

Hello,

I've noticed that the mosquitto library in my Debian repositories (v1.3 - I think) doesn't have the function mosquitto_threaded_set.

The documentation says that if you're using threads you must call this function, but it isn't available in an earlier version that says it's thread safe.

This means that if I wrote a threaded application using v1.3 it would be thread safe, and if I recompiled it using v1.4 it would not be thread safe.

If this is the case, it sounds like a pretty serious bug and potentially a denial-of-service vulnerability.

Cheers,

Andrew

[ralight]

mosquitto_threaded_set is nothing to do with thread safety, it's about how the library optimises its behaviour in multi/single threaded operation.

In a single threaded application, calling mosquitto_publish() will attempt to send your packet immediately as part of the call. In a multi threaded application, we have a thread to deal with the network reading/writing so we do not want mosquitto_publish() to try to send the packet. There's no need, and it is likely that you'll end up lowering performance.

mosquitto_threaded_set simply tells the library you are using your own threads, so it doesn't use the single threaded behaviour. Using mosquitto_loop_start() to run the network thread in a library managed thread works because the library knows that a thread is being used.

[AndrewCarterUK]

Does that mean this comment is inaccurate:

 * Used to tell the library that your application is using threads, but not
 * using <mosquitto_loop_start>. The library operates slightly differently when
 * not in threaded mode in order to simplify its operation. If you are managing
 * your own threads and do not use this function you will experience crashes
 * due to race conditions.

[ralight]

Well that serves me right for going on memory. I trust the me that wrote that at the time I was properly looking at the problem.

A couple of observations:

mosquitto_threaded_set() has been in libmosquitto since 1.3.1. The only versions available in Debian official have been 0.15 and 1.3.4, so your copy should have it.

The other point is that the bug was in version 1.3 and earlier. It's not that 1.3 was thread safe and 1.3.1 onwards isn't, it's that 1.3 incorrectly claimed to be thread safe because it didn't take into account that someone might use their own threads rather than just call mosquitto_loop_start() to do it for them.

What you're asking (in the context of how things work now) is to make external thread mode the default. This has no effect on the people using mosquitto_loop_start(), but will have an impact on people using the single threaded mode. Thinking about it though, calling a load of mosquitto_publish() and only later calling mosquitto_loop_forever() is a bit of an odd thing to do. The more obvious pattern where it could change behaviour is a loop calling mosquitto_loop() which also does publishing. Using mosquitto_loop_start() or mosquitto_loop_forever() are really the preferred options now though, so maybe that isn't a problem.

I think a bit of rumination is in order.

[AndrewCarterUK]

My misunderstanding is that I wasn't aware 1.3 wasn't thread safe if you were using your own threads.

In my application I basically want to have a thread subscribing to a topic and the main thread monitoring a serial port and publishing to a different topic - I'd prefer to manage my own threads (as I have other locking cases and mutex's to deal with) but might look into mosquitto_loop_start(). I was mainly worried that the documentation in the mosquitto.h file I had made it sound like my app was thread safe, but the documentation I could find for the latest version was telling me I wasn't!

I definitely don't have that function on my Debian machine at work though, I'll get some extra information and confirm the version number when I'm back at my desk.

[AndrewCarterUK]

I guess an argument could be made that the library claimed to be thread safe in an earlier version - so that promise should be kept.

But - I only opened the issue because I thought thread-safe functionality had been removed as the default behaviour in 1.3.1 when the function was added.

Happy for it this to be closed if you wish.

[AndrewCarterUK]

Just for the information, I ran 'apt-get install libmosquitto-dev' and ended up with this header file (/usr/include/mosquitto.h):

#define LIBMOSQUITTO_MAJOR 1
#define LIBMOSQUITTO_MINOR 3
#define LIBMOSQUITTO_REVISION 4

However, if I grep the file for mosquitto_threaded_set I get no results.

I also get this compiling:

src/main.cpp: In function ‘int main(int, char**)’:
src/main.cpp:98:36: error: ‘mosquitto_threaded_set’ was not declared in this scope
   mosquitto_threaded_set(mosq, true);

[ralight]

Right well it looks like there is a bug in the change log as to where that feature appeared. It is only in 1.4 onwards.

I'm going to leave this open because I'm still thinking it through.