Hacker News Reader

Comment by AnimalMuppet | Hacker News Reader

An embedded system written in C++ had a Singleton. The Singleton pattern means that you have one instance, which has to live somewhere. This implementation had the instance as a static variable in the getInstance() method, e.g.:

  Foo* getInstance()
  {
    static Foo;
    return foo;
  }

This is perfectly valid.

Unfortunately, getInstance() was implemented in Foo.h, not in Foo.cpp. But in C++, functions in header files are inline by default. This means that every caller of Foo::getInstance() wound up with their own "static Foo" in their own inlined Foo::getInstance(), which meant that each caller got a different instance of the Singleton.

Most interesting bug I've seen in ten years.

View on HN

Love the bug and it's been a while since I've done anything in C++, but isn't your description slightly wrong? There would be one instance per .cpp file that #include'd Foo.h, not one per getInstance() call. I don't think that "inline by default" is an accurate description of what would happen, and I have to imagine that if inline-ing a function caused different behavior, then that would be considered a compiler bug. Rather, I assume you mean it was "inlined" into the file at the #include point (and not once per call). Not that any of that changes the bug qualitatively.

No, inlining replaces it at the place of call. Every call, even if there's multiple of them in the same file. The idea is that, by putting the implementation in the .h file, you're signaling that you don't want to pay the overhead of the function call. (Same thing with the "inline" keyword.)

Now, if your function is sufficiently complicated, the compiler may choose to not inline it anyway (with the definition of "sufficient" being compiler-dependent).

It could be considered a compiler bug that a function with a static variable in the body could be inlined. As you say, that changes the behavior of the function. Or perhaps it could be considered a specification error - I don't know if the standard prohibits inlining in this circumstance, but perhaps it should.

Now, C++ explicitly allows shouting yourself in the foot for performance reasons and for interesting features. You might actually want to have such code to programmatically create such static fields.

In a template tracking usage of the class is a common case. Another fine one would be to count separate instantiations. (How many times the function was used not inlined vs inlined.) Etc.

> A static local variable in an extern inline function always refers to the same object. > 7.1.2/4 - C++98/C++14 (n3797)

(functions are by default extern, so, unless you specifically mark your function as static, this applies to that function)

What the OP did should be fine and not cause an issues. The only way it could cause issues if they also made the function static.

Clarification: Are member functions by default extern?

If so, then it's a compiler bug, and I figured it out because the compiler did what I naively expected, rather than what the standard said.

Now I'm googling this and it looks like what you wrote would be a compiler error (assuming Foo.h was included into at least two files) unless the keyword inline was used so that the same function could be defined multiple times. That clears up my confusion with your "default inline" statement.

Ahh the fabled Multiton pattern. The similar ones are:

Making the singletons in shared objects, each one gets it's own. Adding a destroyInstance method.

I've actually used a destroyInstance method, for testing. But if you do that, first, nobody can hang on to the pointer they get from getInstance, ever, anywhere in the entire code base. And second, the instance has to be a pointer (so that destroyInstance can delete it), and destroyInstance has to NULL the pointer. Then getInstance has to create a new instance if the pointer is NULL.

It can be done, but if anybody ever calls destroyInstance in production code, well, I question their approach...

Reply on news.ycombinator.com