Thanks for the prompt response Roger. My responses are inline...
Ian
----- On Feb 27, 2016, at 4:08 PM, Roger Light <roger@xxxxxxxxxx> wrote:
Hi Ian,
Thanks, that's an interesting email.
> I just recently joined this discussion list for the express purpose of
> bringing Native Windows threading support to libmosquitto on Windows to
> remove the pthreads requirement.
Ok. Is there a particular problem with the current implementations of
pthreads on Windows that you are trying to resolve?
The main problem I am trying to solve is removing the dependency on pthreads on Windows platforms. In my opinion, not having to link against another library, pthread-win32 in this case, is preferable when there is support for threading provided natively. Additionally, most of the pthread implementations are quite old code bases and not well maintained.
> 1. Due to the absence of a pthread_cancel equivalent on Windows, the
> solution I have for scenarios where pthread_cancel would normally be used is
> to call mosquitto_disconnect on the object to force the thread to exit
> gracefully and just use WaitForSingleObject to wait until the thread exits.
Calling mosquitto_disconnect() is something that should only be done
by the end user - it changes the behaviour of the client on the
broker. I'll mention that other pthread implementations have managed
pthread_cancel :)
Right, as I mentioned I only see pthread_cancel used in two functions. One, in mosquitto_loop_stop, with the force variable set to true, and in mosquitto_destroy. I understand that you do not want to directly call mosquitto_disconnect inside mosquitto_destroy, but what about just setting
mosq->state = mosq_cs_disconnecting;
inside mosquitto_destroy? Because this would force the thread to exit gracefully, and then we can wait as I said above with WaitForSingleObject. I see what you are saying about the broker, because mosquitto_disconnect sends a message to the broker, which we don’t want to do, but I don’t see any disadvantages to setting the state to disconnecting to force the thread to gracefully exit. What do you think about this for specifically mosquitto_destroy?
Now, with respect to mosquitto_loop_stop, I am at a bit more of a disadvantage here. I don’t want to have to implement all of pthreads just to get pthread_cancel in this one instance. Before I explore other more exotic methods, I have another question regarding the expected behavior of the mosquitto client when the force variable is set to true in this case. With the pthread_cancel call, my understanding is that whenever the next time that the thread is calling one of these functions (
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_09_05_02 ) then the thread will be killed. Obviously at least one of these functions will be called at some point during mosquitto_loop, but I also notice that right above the call to pthread_cancel in mosquitto_thread_stop is this block of code:
/* Write a single byte to sockpairW (connected to sockpairR) to break out
* of select() if in threaded mode. */
if(mosq->sockpairW != INVALID_SOCKET){
#ifndef WIN32
if(write(mosq->sockpairW, &sockpair_data, 1)){
}
#else
send(mosq->sockpairW, &sockpair_data, 1, 0);
#endif
}
Regardless of how that works, it makes me wonder if a plausible implementation strategy for this might just be to add another state to mosq->state, that’s thread_running or something, and if true it continues to run the thread, if not it exits the thread safely without doing any kind of disconnect sent to the broker, or any other kind of network activity. It could be a similar kind of check as to this at the end of mosquitto_loop_forever :
https://github.com/anonymouse64/mosquitto/blob/master/lib/mosquitto.c#L1058.
> 2. The dummypthread.h file will now be obsolete and replaced by what I have
> tentatively been calling mosquitto_threading.h and mosquitto_threading.c.
If we're assuming that the other windows pthreads implementations
aren't up to scratch, then I think I'd prefer to go down the route of
having code that implemented the pthreads functions in use in
mosquitto. That way the only change needed is to provide that code,
dummypthread.h still works as expected and the broker isn't affected
either.
So you would rather I have a “pthreads.h” and “pthreads.c” file and not define a new type, but rather on windows just provide implementations for pthreads? That’s fine, but personally I view it as more maintainable to be able to change all threading implementations by just changing the implementations of a few functions in one file than a couple hundred lines across a bunch of files iif for some reason down the line there ever becomes a need for some other threading implementation. This also ties the library to the threading model that pthreads uses, so again if further down the line when someone else is maintaining the project and wants to use a different threading model they have to recode lots of parts, rather than just one file. This to me is the same reason why one would have _mosquitto_malloc and such functions.
If there are changes necessary for the broker side, I don’t see much difficulty in configuring it to use mosquitto_thread_t and such, so perhaps if that’s a concern I can contribute changes to that as well.
> I have defined a new struct mosquitto_thread_t and mosquitto_mutex_t that
> use #ifdef WIN32 to determine the type of the underlying object be it
> pthreads or Windows threading handles.
FWIW, I'm not a big fan of adding typedefs for structs just to get rid
of the "struct". There's a good amount of text on the issue here:
http://yarchive.net/comp/linux/typedefs.html
I can't really see why you're bothering with a struct here anyway, why
not just something like the following?
#ifdef WIN32
typedef HANDLE mosquitto_mutex_t;
#else
typedef pthread_mutex_t mosquitto_mutex_t;
#endif
That removes the extra malloc calls you need in some of your functions as well.
I agree, I think this is probably a better implementation strategy. When I started this, I had some justification to myself why not to do this, but I can’t recall what it was now.
> My question here pertains to how important it is that when threading is
> disabled that > the threading components evaluate directly to nothing as they
> do with the > dummypthread.h file.
I suppose it isn't extremely important, but on a matter of principle
I'd like that behaviour retained. It wouldn't be hard to do, it just
needs wider #ifdef clauses in the .c file and some support in the .h.
Understood. This is certainly doable.
> 3. My last point isn't a question so much as a comment. I have been doing my
> development on GitHub (I forked the repo and pushed it up there, you can see
> my work in a dev branch so far here :
> https://github.com/anonymouse64/mosquitto/compare/dev/windows-threading),
Thanks, it's useful to look at what you've done so far.
> and I noticed that there was a post on this thread a few days ago regarding
> migration to GitHub. I am 100% in favor of this; I'm a GitHub fan. I think
> that GitHub has a very good process.
Great, that's good to hear.
> Wolfram Research
So is this a work related project, or something you're doing in your
own time? :)
Yes, I am indeed working on this for work, we are working on integrating MQTT support into our main software product Mathematica.
Cheers,
Roger
_______________________________________________
mosquitto-dev mailing list
mosquitto-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/mosquitto-dev
