-
Notifications
You must be signed in to change notification settings - Fork 5
Description
Yesterday at ECER 2013 our robot crashed multiple times. When I looked at our code today I couldn't find out what was wrong, so I decided to look at the libkovan code... Turns out, our problems were the threads we were using! I only looked through some of the files in the code, however, when looking at something like the following:
void Kovan::enqueueCommand(const Command &command, bool autoFlush)
{
m_queue.push_back(command);
// FIXME: This logic needs to be improved eventually
if(autoFlush) autoUpdate();
}it became apparent that the library is not ready for multi-threading at all! When enqueuing from multiple threads, this becomes a classic read/write race-condition. You should look at this link, which explicitly states that STL containers are not guaranteed to be thread-safe and that it can cause problems when enqueuing from multiple threads and reading from another one (which is exactly what happens here).
Another example would be the following pattern, which is present throughout the kovan library:
Analog *Analog::instance()
{
static Analog s_analog;
return &s_analog;
}Depending on the compiler used, this is also not thread-safe. The C++03 standard stays silent on this matter, so it's not safe to assume that the code the compiler produces will be thread-safe. However, if you are using C++11 then this becomes irrelevant (though I have no reason to believe that you are using C++11 from the code I have seen, because none of the newer features seem to be present).
Until this issue is resolved (and it is very likely going to be a time consuming process to convert the whole codebase to be thread-safe and to sort out all the multithreading bugs) I would advise you to deprecate the threading functions in the libkovan documentation as well as issue a warning not to use any hardware-related functionality from a thread other than the main-thread. The reason to this is that the current documentation mentions nothing of this, which can be irritating for people and lead them to believe that libkovan natively supports multithreading (like we believed... unfortunately, no bugs ever occured before the tournament).