Jump to content

  • Log In with Google      Sign In   
  • Create Account

This singleton keeps crashing.


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
20 replies to this topic

#1 blueshogun96   Crossbones+   -  Reputation: 1096

Like
0Likes
Like

Posted 24 July 2014 - 02:07 AM

I'm not an expert on singletons, and I only have one bit of code that uses it.  It's part of a debugging system from one of NVIDIA's SDKs, and I find  their code to be useful in my engine.  Now, this singleton class works fine in Windows, but not for UNIX based OSes like MacOSX.  This is what it looks like:

#include <cassert>

template <typename T> class Singleton
{
    static T* ms_Singleton;

public:
    Singleton( void )
    {
        assert( !ms_Singleton );
        int offset = *(int*)(T*)1 - *(int*)(Singleton <T>*)(T*)1;
        ms_Singleton = (T*)(*(int*)this + offset);
    }
   virtual ~Singleton( void )
        {  assert( ms_Singleton );  ms_Singleton = 0;  }
    static T& GetSingleton( void )
        {  assert( ms_Singleton );  return ( *ms_Singleton );  }
    static T* GetSingletonPtr( void )
        {  return ( ms_Singleton );  }
};

template <typename T> T* Singleton <T>::ms_Singleton = 0;

So, it crashes in the constructor, starting at the second line.  The windows version had (int) instead of *(int*).  I just changed that so XCode would stop complaining about it.  It could be something simple.  Right now, I'm so tired, I can barely stay awake.  So, I was hoping someone who isn't as braindead as I am right now could point me in the right direction.  Thanks.

 

Shogun.


Follow Shogun3D on the official website: http://shogun3d.net

 

blogger.png twitter.png tumblr_32.png facebook.png

 

"Yo mama so fat, she can't be frustum culled." - yoshi_lol


Sponsor:

#2 Auskennfuchs   Members   -  Reputation: 592

Like
2Likes
Like

Posted 24 July 2014 - 02:42 AM

This *(int*) doesnt work in my opinion, because you subtract the content of the adresses instead of subtract the addresses of the pointers. This little piece of code should just calculates the offset from the Singleton class to the covered class. So in memorylayout the ms_Singleton-member needs its 4 or 8 byte for its pointer-storage. Appended to ms_Singleton the covered-class members are aligned. So to get the right startposition when GetSingleton is used, the pointer is shifted by the calculated offset. Afaik this code was a workaround for older compilers and the newer ones automatically calculate those pointer-shiftings. I would give it a try to just cast the pointer ms_Singleton = (T*)this;

In VS2013 it should probably work, but I haven't tried it on Linux or Mac.

Btw. using int as datatype is critical, because it's size isn't constant through x86 and x64 and different OS. You should use char, short or long.



#3 Hodgman   Moderators   -  Reputation: 31851

Like
9Likes
Like

Posted 24 July 2014 - 04:06 AM

This is a strange design for a Singleton... is this a CRTP base class, where the user is in control of allocation/lifetime?
e.g.

class MyThingy : public Singleton<MyThingy> {};
...
MyThingy globalStorage; // in main/etc
...
MyThingy* test Singleton<MyThingy>::GetSingleton();//anywhere

Regarding your code:

  • *(int*)(T*)1
  • Reinterpret the integer 1 as an T*, reinterpret the T* as an int*, read the integer at that address.
  • Read the integer located at the memory address 0x00000001.

I assume the bytes at that address are not within the valid address space of your process, so you'd get an access violation error!

 

As above, you probably want:
ptrdiff_t offset = ptrdiff_t( ((char*)(T*)0) - ((char*)static_cast<Singleton<T>*>((T*)0)) );
ms_Singleton = (T*)((char*)this + offset);

 

or just:

ms_Singleton = static_cast<T*>(this);


Edited by Hodgman, 24 July 2014 - 04:06 AM.


#4 Tribad   Members   -  Reputation: 887

Like
2Likes
Like

Posted 24 July 2014 - 04:52 AM


Btw. using int as datatype is critical, because it's size isn't constant through x86 and x64 and different OS. You should use char, short or long.

If you needed fixed sizes for your datatypes use stdint.h this header file should be part of the compiler and works in respect to the pre defined macros a compiler issues. The headerfile defines types like int16_t uint32_t and so on. The usage of these types guarantee that the variables/attributes are always the expected size regardless you compile for 32 or 64 bit OS.

 

Only for MS development environments this headerfile was not part of developerstudio. But because I did not use them for a long time it may changed since my last try. But somewhere in the internet you will even find a MS compatible version of stdint.h



#5 Hodgman   Moderators   -  Reputation: 31851

Like
0Likes
Like

Posted 24 July 2014 - 05:56 AM

Yeah, the standard type for an integer that's big enough to store the difference between two pointers, is ptrdiff_t

Only for MS development environments this headerfile was not part of developerstudio. But because I did not use them for a long time it may changed since my last try. But somewhere in the internet you will even find a MS compatible version of stdint.h

recent versions of MSVC include stdint.h, it's only missing in older versions :D

#6 BitMaster   Crossbones+   -  Reputation: 4433

Like
0Likes
Like

Posted 24 July 2014 - 05:58 AM

If you needed fixed sizes for your datatypes use stdint.h [...]


Yes, but if at all possible you should use <cstdint>. Unlike <stdint.h> which might be there or might not be there, <cstdint> is part of the standard library (since C++11).

#7 Wooh   Members   -  Reputation: 652

Like
0Likes
Like

Posted 24 July 2014 - 06:28 AM

&amp;nbsp;

If you needed fixed sizes for your datatypes use stdint.h [...]

Yes, but if at all possible you should use &amp;lt;cstdint&amp;gt;. Unlike &amp;lt;stdint.h&amp;gt; which might be there or might not be there, &amp;lt;cstdint&amp;gt; is part of the standard library (since C++11).
stdint.h is in C++11 too, but I agree with you that cstdint should be preferred.

#8 BitMaster   Crossbones+   -  Reputation: 4433

Like
-3Likes
Like

Posted 24 July 2014 - 06:32 AM

<stdint.h> is part of the C standard library, not the C++ standard library. While most compilers allow you to include their C includes from C++, they are not required to do so (nor do they even have to have C includes somewhere to be standard compliant).

#9 Tribad   Members   -  Reputation: 887

Like
0Likes
Like

Posted 24 July 2014 - 06:54 AM

Anyways where it is. The types defined there are to avoid troubles with the data sizes if something, hardware, depends on it.



#10 Gohliath   Members   -  Reputation: 348

Like
2Likes
Like

Posted 24 July 2014 - 07:46 AM

may help

http://scottbilas.com/publications/gem-singleton/

a little description of that singleton

 

that version got updated already and replaced 

Singleton( void )
{
assert(!ms_singleton);
int offset = (int)(T*)1 - (int)(Singleton *)(T*)1;
ms_singleton = (T*)((int)this + offset);
}

with 

Singleton( void )
{
assert( !ms_singleton );
ms_singleton = static_cast <T*> (this);
}


#11 Wooh   Members   -  Reputation: 652

Like
4Likes
Like

Posted 24 July 2014 - 09:03 AM

<stdint.h> is part of the C standard library, not the C++ standard library. While most compilers allow you to include their C includes from C++, they are not required to do so (nor do they even have to have C includes somewhere to be standard compliant).


This is what the C++11 standard (Appendix D.5) says about it.
 

For compatibility with the C standard library and the C Unicode TR, the C++ standard library provides the 25 C headers, as shown in Table 154.

Table 154 - C headers
<assert.h><inttypes.h><signal.h><stdio.h><wchar.h>
<complex.h><iso646.h><stdalign.h><stdlib.h><wctype.h>
<ctype.h><limits.h><stdarg.h><string.h>
<errno.h><locale.h><stdbool.h><tgmath.h>
<fenv.h><math.h><stddef.h><time.h>
<float.h><setjmp.h><stdint.h><uchar.h>



#12 BitMaster   Crossbones+   -  Reputation: 4433

Like
0Likes
Like

Posted 24 July 2014 - 09:10 AM

I concede the point, but I would still strongly recommend never to include C headers instead of their C++ counterpart unless an extremely compelling reason exists.

Edited by BitMaster, 24 July 2014 - 09:11 AM.


#13 blueshogun96   Crossbones+   -  Reputation: 1096

Like
0Likes
Like

Posted 24 July 2014 - 05:22 PM

Okay, the responses *sorta* fixed the issue I've been having with the singleton.  It doesn't crash anymore, but now there's something else going wrong.  Whenever the singleton class actually gets used, it crashes, but somewhere else in the program (an internal crash within SDL_InitSubSystem(SDL_INIT_VIDEO)).  And honestly, I have no idea what's going on here.  Maybe I should have shared the debugging class while I was at it.

/******************************************************************************

  Copyright (C) 1999, 2000 NVIDIA Corporation
  This file is provided without support, instruction, or implied warranty of any
  kind.  NVIDIA makes no guarantee of its fitness for a particular purpose and is
  not liable under any circumstances for any damages or loss whatsoever arising
  from the use or inability to use this file or items derived from it.
  
    Comments:
    
	Simple debug support with stream input for complex arguments.
	Also writes to a debug log style file.

	Declare this somewhere in your .cpp file:

	NVDebug dbg(DebugLevel, "outfile.txt")
	
	// Output Like this 
	DISPDBG(0, "This" << " is " << "a useful debug class");

  cmaughan@nvidia.com
      
******************************************************************************/
#ifndef __NVDEBUG_H
#define __NVDEBUG_H

#ifdef _WIN32
#include <windows.h>
#pragma warning(disable: 4786)
#include <tchar.h>
#endif
#pragma warning(disable: 4786)
#include <iostream>
#pragma warning(disable: 4786)
#include <sstream>
#pragma warning(disable: 4786)
#include <iomanip>
#pragma warning(disable: 4786)
#include <strstream>
#pragma warning(disable: 4786)
#include <fstream>
#pragma warning(disable: 4786)
#include <assert.h>
#include "singleton.h"
#ifdef _WIN32
#include <crtdbg.h>
#endif

/* Non-Windows platforms */
#ifndef _WIN32
#define OutputDebugString printf
#define TEXT(s) s
#endif

/* Trap functionality */
#ifdef _MSC_VER
#define __trap  _asm int 3
#else
#define __tram  __asm__("int $3")
#endif


static const DWORD MaxDebugFileSize = 100000;

class NVDebug : public Singleton<NVDebug>
{
public:

	NVDebug(unsigned int GlobalDebugLevel, const char* pszFileName)
		: m_GlobalDebugLevel(GlobalDebugLevel),
		m_dbgLog(pszFileName, std::ios::out | std::ios::trunc)	// Open a log file for debug messages
	{
		OutputDebugString(TEXT("NVDebug::NVDebug\n"));
	}
	
	virtual ~NVDebug()
	{
		m_dbgLog.flush();
		m_dbgLog.close();

		m_strStream.flush();
		m_strStream.clear();
	}

	// Flush the current output
#ifdef _WIN32
	void NVDebug::EndOutput()
#else
    void EndOutput()
#endif
	{
		m_strStream << std::endl << std::ends;

		// Don't make a huge debug file.
		if (m_dbgLog.tellp() < MaxDebugFileSize)
		{
			m_dbgLog << m_strStream.str();
			FlushLog();
		}

		OutputDebugString(m_strStream.str());

		m_strStream.freeze(false);
		m_strStream.seekp(0);
	}

	unsigned int GetLevel() const { return m_GlobalDebugLevel; };
	std::ostrstream& GetStream() { return m_strStream; }
	void FlushLog() { m_dbgLog.flush(); }
	
private:
	std::ostrstream m_strStream;
	std::ofstream m_dbgLog;
	unsigned int m_GlobalDebugLevel;
};

#ifdef _DEBUG
#define DISPDBG(a, b)											\
do																\
{																\
	if (NVDebug::GetSingletonPtr() != NULL)						\
	if (a <= NVDebug::GetSingleton().GetLevel()) {	\
		NVDebug::GetSingleton().GetStream() << b;			\
		NVDebug::GetSingleton().EndOutput(); }				\
} while(0)

#define NVASSERT(a, b)														\
do																			\
{																			\
	static bool bIgnore = false;											\
	if (!bIgnore && ((int)(a) == 0))										\
	{																		\
		std::ostringstream strOut;											\
		strOut << b << "\nAt: " << __FILE__ << ", " << __LINE__;			\
		int Ret = MessageBoxEx(NULL, strOut.str().c_str(), "NVASSERT", MB_ABORTRETRYIGNORE, MAKELANGID( LANG_NEUTRAL, SUBLANG_DEFAULT ));	\
		if (Ret == IDIGNORE)			\
		{								\
			bIgnore = true;				\
		}								\
		else if (Ret == IDABORT)		\
		{								\
            __trap;                     \
		}								\
	}									\
} while (0)

#else
#define DISPDBG(a, b)
#define NVASSERT(a, b)
#endif

// Note that the cast ensures that the stream
// doesn't try to interpret pointers to objects in different ways.
// All we want is the object's address in memory.
#define BASE_DBG_PTR(a)					\
"0x" << std::hex << (DWORD)(a) << std::dec
	
#endif // __NVDEBUG_H

I hacked it up a bit so it would compile under XCode (and yeah, I know I still have to deal with the MessageBox code; SDL has a rough equivalent), and so far, I've only used the example from the top comment in this header.  The code works, and the string is placed in the log file, but after that, that totally unrelated crash happens shortly after.  Doesn't make any sense to me, or was a pointer corrupted somehow?  I dunno what this singleton class is actually doing, or even what makes it useful.  Any more ideas?  Thanks.

 

Shogun.

 

EDIT: I forgot to mention that if I comment out DISPDBG in my code, no crashes.

 

//DISPDBG(0, "This" << " is " << "a useful debug class"); <- No crash


Edited by blueshogun96, 24 July 2014 - 05:25 PM.

Follow Shogun3D on the official website: http://shogun3d.net

 

blogger.png twitter.png tumblr_32.png facebook.png

 

"Yo mama so fat, she can't be frustum culled." - yoshi_lol


#14 ApochPiQ   Moderators   -  Reputation: 16402

Like
2Likes
Like

Posted 24 July 2014 - 05:28 PM

Mandatory reading.

#15 blueshogun96   Crossbones+   -  Reputation: 1096

Like
0Likes
Like

Posted 24 July 2014 - 06:24 PM

Okay, here's a better set of details.

 

The exact message I get from XCode is this: "Thread 1 (Main thread, obviously): EXC_BAD_ACCESS (code=13, address=0x0)"

 

Inside of SDL_InitSubSystem( SDL_INIT_VIDEO ), there is a crash inside of a call to getMethodNoSuper_nolock(class_t*, objc_selector*).  Also, this crash doesn't even happen in the same function where the debug code was initialized.  Only one call to DISPDBG was called, and it was directly after initializing the class with the singleton.

 

I've tried this:

NVDebug dbg( 1, "debug.txt" );

 

And I've tried this:

NVDebug* dbg = NVDebug( 1, "debug.txt" );

 

Immediately after, I do this:

DISPDBG( 1, "Initialization started...\n" );

 

Now, the crash is actually quite far from this initialization code.  There doesn't really appear to be much I can do to find out exactly whats causing it.  Maybe I'm asking for too much, sorry about that.  I'm just unsure why this is happening on MacOSX, and not Windows.

 

Shogun.


Follow Shogun3D on the official website: http://shogun3d.net

 

blogger.png twitter.png tumblr_32.png facebook.png

 

"Yo mama so fat, she can't be frustum culled." - yoshi_lol


#16 ApochPiQ   Moderators   -  Reputation: 16402

Like
0Likes
Like

Posted 24 July 2014 - 07:31 PM

Post the complete call stack and indicate where your code begins in that stack.

#17 blueshogun96   Crossbones+   -  Reputation: 1096

Like
0Likes
Like

Posted 24 July 2014 - 07:46 PM

I took two screen shots of the call stack.  The first one:

 

Screen Shot 2014-07-24 at 6.38.31 PM.png

 

Not much here, because this is just one of my functions called within the main function.  This is where my singleton is initialized.  After I finish with this function we're in now, I leave that one (return from it), and call the next one.

 

Screen Shot 2014-07-24 at 6.39.30 PM.png

 

This is where things no longer make sense.  If it's going to crash, why there of all places?  Where the code began is not in this second image, but the first one, so it's really confusing.

 

Shogun.


Follow Shogun3D on the official website: http://shogun3d.net

 

blogger.png twitter.png tumblr_32.png facebook.png

 

"Yo mama so fat, she can't be frustum culled." - yoshi_lol


#18 ApochPiQ   Moderators   -  Reputation: 16402

Like
0Likes
Like

Posted 24 July 2014 - 09:22 PM

Not sure why you think this is in any way related to your singleton. It's most likely a problem in Objective-C code, specifically poking an object that doesn't exist (either null or has been released).


Also that callstack looks incomplete, although I honestly am not familiar enough with Xcode to know how to get the whole thing.

Edited by ApochPiQ, 24 July 2014 - 09:28 PM.


#19 blueshogun96   Crossbones+   -  Reputation: 1096

Like
0Likes
Like

Posted 25 July 2014 - 12:38 AM

After much tedious work, I think it's safe to say that it's not the singleton.  I tried removing it and using a much less convenient way of doing things, and it still crashes.  At least I have a better idea of what's going on.  Here's the error:

 

"example03(21782,0x7fff73dd2960) malloc: *** error for object 0x10181a8e8: incorrect checksum for freed object - object was probably modified after being freed.

*** set a breakpoint in malloc_error_break to debug"

 

Something wrong with the STL code.  There's a buffer overflow most likely.

 

Shogun.

 

EDIT: Fixed it.  I looked up std::ostrstring, and it's a deprecated string type and has been since C++98.  Changing to std::ostringstream fixed it.  Now I feel stupid... again.  I owe singletons everywhere an apology.


Edited by blueshogun96, 25 July 2014 - 01:03 AM.

Follow Shogun3D on the official website: http://shogun3d.net

 

blogger.png twitter.png tumblr_32.png facebook.png

 

"Yo mama so fat, she can't be frustum culled." - yoshi_lol


#20 SeanMiddleditch   Members   -  Reputation: 7191

Like
0Likes
Like

Posted 25 July 2014 - 07:24 PM

This singleton design is weird. Scott Bilas' article is from 2000; there are better ways to do this today. A better way (other than just not using singletons which is an even better idea):
 
// note that this does not attempt to be thread-safe as it is assumed you do initialization/destruction
// early in program startup and late during shutdown when no threads will be active
template <typename T>
class ManagedSingleton {
// where we allocate the singleton object - guaranteed to be big enough and aligned properly
std::aligned_storage<sizeof(T), alignof(T)>::type _storage;

// basically just a flag and a convenience to see if the singleton is engaged (has a valid object in it) or not
T* _ptr = nullptr;

public:
// disable copies of the singleton container
ManagedSingleton (ManagedSingleton const&) = delete;
void operator=(ManagedSingleton  const&) = delete;

// create a new instance of some type which is either T or a derived type of T
template <typename C, typename... Params>
C* Create(Params&&... params) {
assert(_ptr == nullptr);
new (&_storage) C(std::forward<Params>(params)...);
_ptr = reinterpret_cast<T*>(&_storage);
return static_cast<C*>(_ptr);
}

// destroy the instance
void Destroy() {
assert(_ptr != nullptr);
_ptr->~T();
_ptr = nullptr;
}

// check if we have a valid object or not
bool empty() const { return _ptr == nullptr; }

// access the object
T* get() const { assert(_ptr != nullptr); return _ptr; }
T* operator->() const { return get(); }
T& operator*() const { return &get(); }
};
// exmaple using it
class IFooSystem {
protected:
virtual ~IFooSystem() {}
};
class FooSystem : public IFooSystem {
public:
FooSystem(int arg1, float arg2, char arg3);

void DoFoo();
};

ManagedSingleton<IFooSystem> g_fooSys;

int main() {
g_fooSys.Create<FooSystem>(1, 2.0f, '3');

g_fooSys->DoFoo();

g_fooSys.Destroy();
}
(I have no idea why the forum ate the indentation; it usually doesn't do that)

This explicitly contains the storage location, doesn't require you to declare the static storage separately, lets you have managed singletons of interfaces rather than of concrete types, doesn't require that you declare a type as a singleton explicitly, has no weird pointer manipulation, and in release mode is as fast as a global pointer.

Edited by SeanMiddleditch, 25 July 2014 - 07:26 PM.





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS