aboutsummaryrefslogtreecommitdiffstatshomepage
path: root/_posts/2015-07-03-std-call-once-bug.md
diff options
context:
space:
mode:
authorEgor Tensin <Egor.Tensin@gmail.com>2017-01-31 18:38:17 +0300
committerEgor Tensin <Egor.Tensin@gmail.com>2017-01-31 18:38:17 +0300
commit966a2b2326ccdbc058fa01574bbe3e5bd6c1fd9a (patch)
treeaf57f3416a79c66d00b674b00be909d9318f216b /_posts/2015-07-03-std-call-once-bug.md
parent_posts: grammar tweaks (diff)
downloadblog-966a2b2326ccdbc058fa01574bbe3e5bd6c1fd9a.tar.gz
blog-966a2b2326ccdbc058fa01574bbe3e5bd6c1fd9a.zip
std::call_once: shorten title
Diffstat (limited to '_posts/2015-07-03-std-call-once-bug.md')
-rw-r--r--_posts/2015-07-03-std-call-once-bug.md270
1 files changed, 270 insertions, 0 deletions
diff --git a/_posts/2015-07-03-std-call-once-bug.md b/_posts/2015-07-03-std-call-once-bug.md
new file mode 100644
index 0000000..0fd6c30
--- /dev/null
+++ b/_posts/2015-07-03-std-call-once-bug.md
@@ -0,0 +1,270 @@
+---
+title: std::call_once bug in Visual Studio 2012/2013
+layout: post
+excerpt: >
+ In this post I will describe a nasty bug I've stumbled upon in the C++
+ Standard Library implementation shipped with Microsoft Visual Studio
+ 2012/2013.
+---
+I've recently come across a nasty standard library bug in the implementation
+shipped with Microsoft Visual Studio 2012/2013.
+[StackOverflow was of no help], so I had to somehow report the bug to the
+maintainers.
+Oddly enough, Visual Studio's [Connect page] wouldn't let me report one,
+complaining about the lack of permissions, even though I was logged in from my
+work account, associated with my Visual Studio 2013 installation.
+
+Fortunately, I've come across the personal website of this amazing guy,
+[Stephan T. Lavavej], who appears to be the chief maintainer of Microsoft's
+standard library implementation.
+He seems to be your go-to guy when it comes to obvious standard library
+misbehaviours.
+
+[StackOverflow was of no help]: https://stackoverflow.com/questions/26477070/concurrent-stdcall-once-calls
+[Connect page]: https://connect.microsoft.com/VisualStudio
+[Stephan T. Lavavej]: http://nuwen.net/stl.html
+
+C++11 and singletons
+--------------------
+
+Anyway, the story begins with me trying to implement the singleton pattern
+using C++11 facilities like this:
+
+```
+#include <mutex>
+
+template <typename Derived>
+class Singleton
+{
+public:
+ static Derived& get_instance()
+ {
+ std::call_once(initialized_flag, &initialize_instance);
+ return Derived::get_instance_unsafe();
+ }
+
+protected:
+ Singleton() = default;
+ ~Singleton() = default;
+
+ static Derived& get_instance_unsafe()
+ {
+ static Derived instance;
+ return instance;
+ }
+
+private:
+ static void initialize_instance()
+ {
+ Derived::get_instance_unsafe();
+ }
+
+ static std::once_flag initialized_flag;
+
+ Singleton(const Singleton&) = delete;
+ Singleton& operator=(const Singleton&) = delete;
+};
+
+template <typename Derived>
+std::once_flag Singleton<Derived>::initialized_flag;
+```
+
+Neat, huh?
+Now other classes can inherit from `Singleton`, implementing the singleton
+pattern effortlessly:
+
+```
+class Logger : public Singleton<Logger>
+{
+private:
+ Logger() = default;
+ ~Logger() = default;
+
+ friend class Singleton<Logger>;
+};
+```
+
+Note that the [N2660] standard proposal isn't/wasn't implemented in the
+compilers shipped with Visual Studio 2012/2013.
+If it was, I wouldn't, of course, need to employ this `std::call_once`
+trickery, and the implementation would be much simpler, i.e. something like
+this:
+
+```
+class Logger
+{
+public:
+ static Logger& get_instance()
+ {
+ static Logger instance;
+ return instance;
+ }
+
+private:
+ Logger() = default;
+ ~Logger() = default;
+};
+```
+
+<div class="alert alert-info" markdown="1">
+
+The point is that the `Logger::get_instance` routine above wasn't thread-safe
+until C++11.
+Imagine what might happen if `Logger`s constructor takes some time to
+initialize the instance.
+If a couple of threads then call `get_instance`, the first thread might begin
+the initialization process, making the other thread believe that the instance
+had already been intialized.
+This other thread might then return a reference to the instance which hasn't
+yet completed its initialization and is most likely unsafe to use.
+
+Since C++11 includes the proposal mentioned above, this routine would indeed be
+thread-safe in C++11.
+Unfortunately, the compilers shipped with Visual Studio 2012/2013 don't/didn't
+implement this particular proposal, which caused me to look at
+`std::call_once`, which seemed to implement exactly what I needed.
+
+</div>
+
+[N2660]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2660.htm
+
+Problem
+-------
+
+Unfortunately, matters became a bit more complicated when I tried to introduce
+two singletons, one having a dependency on the other.
+I had `Logger`, like in the example above, and some kind of a "master"
+singleton (let's call it `Duke`).
+`Duke`'s constructor was complicated and time-consuming, and definetely
+required some logging to be done.
+I thought that I could simply call `Logger::get_instance` inside `Duke`'s
+constructor, and everything looked fine at first glance.
+
+```
+#include <chrono>
+#include <thread>
+
+class Logger : public Singleton<Logger>
+{
+public:
+ Logger& operator<<(const char* msg)
+ {
+ // Actual logging is stripped for brevity.
+ return *this;
+ }
+
+private:
+ Logger()
+ {
+ // Opening log files, etc.
+ std::this_thread::sleep_for(std::chrono::seconds{3});
+ }
+
+ ~Logger() = default;
+
+ friend class Singleton<Logger>;
+};
+
+class Duke : public Singleton<Duke>
+{
+private:
+ Duke()
+ {
+ Logger::get_instance() << "started Duke's initialization";
+ // It's a lot of work to be done.
+ std::this_thread::sleep_for(std::chrono::seconds{10});
+ Logger::get_instance() << "finishing Duke's initialization";
+ }
+
+ ~Duke() = default;
+
+ friend class Singleton<Duke>;
+};
+```
+
+Now, what happens if I have two threads, one using the `Duke` instance, and the
+other logging something?
+Like in this example:
+
+```
+#include <thread>
+
+namespace
+{
+ void get_logger()
+ {
+ entered(__FUNCTION__);
+ Logger::get_instance();
+ exiting(__FUNCTION__);
+ }
+
+ void get_duke()
+ {
+ entered(__FUNCTION__);
+ Duke::get_instance();
+ exiting(__FUNCTION__);
+ }
+}
+
+int main()
+{
+ std::thread t1{&get_duke};
+ std::thread t2{&get_logger};
+ t1.join();
+ t2.join();
+ return 0;
+}
+```
+
+`entered` and `exiting` are utility functions to print timestamps.
+The implementation is included in the [complete code sample].
+{: .alert .alert-info}
+
+The first thread is supposed to have the total running time of about 13
+seconds, right?
+Three seconds to initialize the `Logger` instance and ten to initialize the
+`Duke` instance.
+The second thread, similarly, is supposed to be done in about 3 seconds
+required for the initialization of `Logger`.
+
+Weirdly, this program produces the following output when compiled using Visual
+Studio 2013's compiler:
+
+ Entered `anonymous-namespace'::get_duke at Fri Jul 03 02:26:16 2015
+ Entered `anonymous-namespace'::get_logger at Fri Jul 03 02:26:16 2015
+ Exiting `anonymous-namespace'::get_duke at Fri Jul 03 02:26:29 2015
+ Exiting `anonymous-namespace'::get_logger at Fri Jul 03 02:26:29 2015
+
+Isn't it wrong that the second thread actually took the same 13 seconds as the
+first thread?
+Better check with some other compiler in case it was me who made a mistake.
+Unfortunately, the program behaves as expected when compiled using GCC:
+
+ Entered get_logger at Fri Jul 3 02:27:12 2015
+ Entered get_duke at Fri Jul 3 02:27:12 2015
+ Exiting get_logger at Fri Jul 3 02:27:15 2015
+ Exiting get_duke at Fri Jul 3 02:27:25 2015
+
+So it appears that the implementation of `std::call_once` shipped with Visual
+Studio 2012/2013 relies on some kind of a global lock, which causes even the
+simple example above to misbehave.
+
+The [complete code sample] to demonstrate the misbehaviour described above can
+be found in this blog's repository.
+
+[complete code sample]: {{ site.github.repository_url }}/tree/gh-pages/src/posts/std_call_once_bug_in_visual_studio_2012_2013
+
+Resolution
+----------
+
+So, since I couldn't submit the bug via Visual Studio's [Connect page], I wrote
+to Mr. Lavavej directly, not hoping for an answer.
+Amazingly, it took him less than a day to reply.
+He told me he was planning to overhaul `std::call_once` for Visual Studio 2015.
+Meanwhile, I had to stick to something else; I think I either dropped logging
+from `Duke`'s constructor or initialized all the singleton instances manually
+before actually using any of them.
+In a few months, Mr. Lavavej replied to me that the bug has been fixed in
+Visual Studio 2015 RTM.
+I would like to thank him for the professionalism and responsibility he's
+shown.