Archived

This topic is now archived and is closed to further replies.

level10boy

More Meyers Singleton woes with VC++

Recommended Posts

Ok, I''ve bit the bullet and made the destructor public but now I have a new problem. Everything is fine until I hit Release build with maximum speed optimizations turned on. The constructor for my Meyers singletons gets called twice, which I find hard to believe as it doesn''t happen with any other build type. The object basically gets constructed twice. I must say I cannot see why this is hapenning and am totally mistified. Any advice or answers will be much appreciated.

Share this post


Link to post
Share on other sites
It''s quite simple actually. When you inline a method,
the actual code is copy & pasted by the compiler to
replace the function call. So if your getInstance() is
copied twice or more, you''ll get many instances of your
"singleton".

In general, it''s a BAD idea to inline methods that
contain static data for exactly that reason.



Kami no Itte ga ore ni zettai naru!

Share this post


Link to post
Share on other sites
In case somebody doesn't know what I was talking about in
the previous post, I forgot to mention that the Meyers
Singleton is usually implemented as:


    
// In Singleton.h

class Singleton
{
public:
static Singleton &getInstance(void);
}
// end


// In Singleton.cpp

Singleton &Singleton::getInstance(void)
{
static Singleton s_Singleton;
return s_Singleton;
}
// end



[Edit: changed getInstance() to static]



Kami no Itte ga ore ni zettai naru!

[edited by - tangentz on July 29, 2002 10:31:22 AM]

Share this post


Link to post
Share on other sites
quote:
Original post by tangentz
It''s quite simple actually. When you inline a method,
the actual code is copy & pasted by the compiler to
replace the function call. So if your getInstance() is
copied twice or more, you''ll get many instances of your
"singleton".

In general, it''s a BAD idea to inline methods that
contain static data for exactly that reason.



That is close but not quite right. The problem is not with the inline, but static.

Static variables inside inline functions share the same variable throughout the whole program. (section 7.1.4.2)

Static functions ( deprecated now, should be functions in unnamed namespaces) are defined inside each translation unit. Therefore, the static keyword is causing the multiple definition of the function.

Consider this.

inline void Shared()
{
static int shared; // external linkage, whole program share same instance
}

// note: deprecated, use unnamed namespace instead
static void Local()
{
static int local; // internal linkage, every cpp file have an instance of this
}


For a inlined static class member function, the linkage should be external. Unless you are using an very old compiler which is confused by the inline and static but I don''t think VC++ 6 is one of them. Perhaps level10boy who like to paste his test case.

Ps. Your meyer singleton is wrong. I believe you need to declare getInstance as a static member function, unless you are showing a different variant.

Share this post


Link to post
Share on other sites
static works differently on member functions than on global functions, and there aren''t any global functions here. I would have thought the compiler would have been smart enough to realise that the function had static variables and would refuse to inline it though...

If I had my way, I''d have all of you shot!


codeka.com - Just click it.

Share this post


Link to post
Share on other sites
quote:
Original post by Void
That is close but not quite right. The problem is not with the inline, but static.



Err, but the problem is inline and static.

quote:

Static variables inside inline functions share the same variable throughout the whole program. (section 7.1.4.2)



True, in theory. But some "popular" compilers don''t conform
to this. Scott Meyers talks about it in Effective C++ and
More Effective C++. I know this because I experienced the
exact same thing. We''re talking VC++ here...

quote:

Consider this.
inline void Shared()
{
static int shared; // external linkage, whole program share same instance
}



Not if the compiler is "faulty". Shared() is copied as
many times as it is inlined, causing multiple instances
of the static variables to be present. See MEC++. The only
way to be sure is to make Shared() non-inline.

quote:

// note: deprecated, use unnamed namespace instead
static void Local()
{
static int local; // internal linkage, every cpp file have an instance of this
}



Sorry, but this is wrong. A static function exists ONLY
in the translation unit (CPP) where it is compiled in. You
can''t even extern and use it, because it has internal linkage.
Only ONE instance of this is present.

quote:

For a inlined static class member function, the linkage should be external. Unless you are using an very old compiler which is confused by the inline and static but I don''t think VC++ 6 is one of them. Perhaps level10boy who like to paste his test case.



Unfortunately, VC++ 6 is one of them.



Kami no Itte ga ore ni zettai naru!

Share this post


Link to post
Share on other sites
quote:
Original post by Dean Harding
static works differently on member functions than on global functions, and there aren''t any global functions here. I would have thought the compiler would have been smart enough to realise that the function had static variables and would refuse to inline it though...



VC++ 6 (don''t know about 7) would gladly inline functions with
static variables (BAD idea!).





Kami no Itte ga ore ni zettai naru!

Share this post


Link to post
Share on other sites
quote:
Original post by tangentz
VC++ 6 (don''t know about 7) would gladly inline functions with
static variables (BAD idea!).



Wow, I never knew that I''ll try it out on VS.NET later on and see if it''s fixed...

If I had my way, I''d have all of you shot!


codeka.com - Just click it.

Share this post


Link to post
Share on other sites
While we're on the subject, would using a static class member variable be O.K. as opposed to a static variable inside a member function?


      
// cA.h


class A
{
public:
~A(void)
{
if(mInstance)
delete mInstance;
}

public:
inline static A &Instance(void)
{
if(!mInstance)
mInstance = new A;
return *mInstance;
}

private:
A(void);
static A* mInstance;
};

// cA.cpp

A::A() {}

A* A::mInstance = 0;


This thread just royally confused me I could just move the Instance code outside the class declaration to avoid inlining it (and removing the 'inline' keyword, duh), but I need to know if this is fine!

[edited by - Zipster on July 29, 2002 6:23:39 PM]

Share this post


Link to post
Share on other sites
Zipster, that is fine. Only one instance of the static
class member variable is ever instantiated in whichever CPP
file you define it. A static class member variable is
like extern , but you stuff it under the class
rather than leaving it in the global namespace.

Let me say it again. inline function(s) with
static variable(s) is a BAD idea. Some compilers
(e.g. VC++ 6) just can''t deal with it and result in multiple
copies of the static variable(s) being instantiated. The
only way to be sure is to NEVER inline functions with
static variables.

P.S. This is taken from, appropriately enough, Scott Meyers
pair of books -- Effective C++ and More Effective C++.



Kami no Itte ga ore ni zettai naru!

Share this post


Link to post
Share on other sites
quote:
Original post by tangentz
Not if the compiler is "faulty". Shared() is copied as
many times as it is inlined, causing multiple instances
of the static variables to be present. See MEC++. The only
way to be sure is to make Shared() non-inline.



I agree. The 1996 C++ standard corrects this behaviour. If it was a pre 1996 compiler, I would expect it to have this problem. But VC++6 ? (Well, I''m won''t too surprised if it has that bug)

quote:

Sorry, but this is wrong. A static function exists ONLY
in the translation unit (CPP) where it is compiled in. You
can''t even extern and use it, because it has internal linkage.
Only ONE instance of this is present.



I am assuming you put it in a header file like the inlined version.

Take a look at this code( VC++ 6sp5 - full optimizations, inlined whereever suitable)


  
// %%%%%% C.h %%%%%

#include <iostream>

class C
{
public:
static void Method();
};

inline void C::Method()
{
static int count;
std::cout << ++count << "\n";
}

//%%%%% another.cpp %%%%%

#include "C.h"

void F()
{
C::Method(); // breakpoint

}

//%%%%% main.cpp %%%%%%

#include "C.h"

extern void F();

int main()
{
C::Method();
F(); // breakpoint

return 0;
}


The output printed is 1,2. As expected.

Looking at the disassembly



//%%%%% main.cpp %%%%%

5: int main()
6: {
00401000 mov eax,[count (0041ef84)]
00401005 push offset string "\n" (0041b070)
0040100A inc eax
0040100B mov ecx,offset std::cout (0041eb38)
00401010 push eax
00401011 mov [count (0041ef84)],eax
00401016 call std::basic_ostream >::operator<< (00401030)
0040101B push eax
0040101C call std::operator<< (00401490)
00401021 add esp,8
7: C::Method();
8:
9: F();
00401024 call F (00403970)
10:
11: return 0;
00401029 xor eax,eax
12: }
0040102B ret


// %%%%% another.cpp %%%%%
3: void F()
4: {
00403970 mov eax,[count (0041ef84)]
00403975 push offset string "\n" (0041b070)
0040397A inc eax
0040397B mov ecx,offset std::cout (0041eb38)
00403980 push eax
00403981 mov [count (0041ef84)],eax
00403986 call std::basic_ostream >::operator<< (00401030)
0040398B push eax
0040398C call std::operator<< (00401490)
00403991 add esp,8
5: C::Method();
6: }
00403994 ret


C::Method() looks inlined to me and it produces the correct results.

You can try it with a inlined non member function and it still shows the correct results.


  
//%%%% C.h %%%

#include <iostream>

// if this is static, then it is a totally different story

inline void C()
{
static int count;
std::cout << ++count << "\n";
}

//%%%% another.cpp %%%%

#include "C.h"

void F()
{
C();
}

//%%%%% main.cpp %%%%

#include "C.h"

extern void F();

int main()
{
C();
F();
return 0;
}

[/source]

Share this post


Link to post
Share on other sites
Try a third variation: Put the function calling the inline
method in question in a separate static library and link the
main EXE with the .lib and see what happens. Actually,
why don''t we make 3 or more of "F()" and have some fun?



Kami no Itte ga ore ni zettai naru!

Share this post


Link to post
Share on other sites
quote:
Original post by tangentz
Try a third variation: Put the function calling the inline
method in question in a separate static library and link the
main EXE with the .lib and see what happens. Actually,
why don''t we make 3 or more of "F()" and have some fun?



Something must be wrong with my VC++ 6 . It produces the correct results everytime. Perhaps you like to send me your test case.

Share this post


Link to post
Share on other sites