Sign in to follow this  
Mybowlcut

[SDL] Weird exception

Recommended Posts

Mybowlcut    176
Hey. I've got an exception that I can't figure out... Unhandled exception at 0x1001fc79 in Earth.exe: 0xC0000005: Access violation reading location 0xbaad0034.
// Wrapper class for SDL_Surface that provides Copy-on-write semantics.
class Surface
{
public:
	Surface() {}
	Surface(const std::string& file_name, SDL_Colour* colour_key = NULL);

	Surface& operator=(const Surface& rhs);

	void Draw(Renderer& renderer, SDL_Rect position, SDL_Rect* clip = NULL);

	Uint16 Width() const;
	Uint16 Height() const;
private:
	class Inner_Surface : public boost::noncopyable
	{
	public:
		Inner_Surface() : surface(NULL) {}
		Inner_Surface(SDL_Surface* surface) : surface(surface) {}
		~Inner_Surface() { SDL_FreeSurface(surface); } // points to this line
		
		SDL_Surface* surface;
	};

	boost::shared_ptr<Inner_Surface> inner;
};


My call stack looks like this:
Quote:
SDL.dll!1001fc79() [Frames below may be incorrect and/or missing, no symbols loaded for SDL.dll] SDL.dll!100200a0() SDL.dll!10026c7f() > Earth.exe!Surface::Inner_Surface::~Inner_Surface() Line 36 + 0x2e bytes C++ Earth.exe!Surface::Inner_Surface::`scalar deleting destructor'() + 0x2b bytes C++ Earth.exe!boost::checked_delete<Surface::Inner_Surface>(Surface::Inner_Surface * x=0x00b3bc48) Line 34 + 0x2b bytes C++ Earth.exe!boost::detail::sp_counted_impl_p<Surface::Inner_Surface>::dispose() Line 79 + 0xc bytes C++ Earth.exe!boost::detail::sp_counted_base::release() Line 102 + 0xf bytes C++ Earth.exe!boost::detail::shared_count::~shared_count() Line 209 C++ Earth.exe!boost::shared_ptr<Surface::Inner_Surface>::~shared_ptr<Surface::Inner_Surface>() + 0x2e bytes C++ Earth.exe!Surface::~Surface() + 0x2b bytes C++ Earth.exe!XImage::~XImage() Line 33 + 0xb bytes C++ Earth.exe!XImage::`scalar deleting destructor'() + 0x2b bytes C++ Earth.exe!XScreen<Type_List<XImage,Type_List<XButton,Type_List<XSlider,Type_List<XDialog_Box,Null_Type> > > > >::~XScreen<Type_List<XImage,Type_List<XButton,Type_List<XSlider,Type_List<XDialog_Box,Null_Type> > > > >() Line 76 + 0x3b bytes C++ Earth.exe!Earth::~Earth() Line 35 + 0xa8 bytes C++ msvcr80d.dll!_CallSettingFrame(unsigned long funclet=1244980, unsigned long pRN=259, unsigned long dwInCode=939933583) Line 73 Asm msvcr80d.dll!__FrameUnwindToState(EHRegistrationNode * pRN=0x0012ff34, void * pDC=0x0012eec4, const _s_FuncInfo * pFuncInfo=0x004bd598, int targetState=2) Line 1154 C++ msvcr80d.dll!CatchIt(EHExceptionRecord * pExcept=0x0012eef0, EHRegistrationNode * pRN=0x0012ff34, _CONTEXT * pContext=0x0012ef10, void * pDC=0x0012eec4, const _s_FuncInfo * pFuncInfo=0x004bd598, const _s_HandlerType * pCatch=0x004bd578, const _s_CatchableType * pConv=0x004bc08c, const _s_TryBlockMapEntry * pEntry=0x004bd5d0, int CatchDepth=0, EHRegistrationNode * pMarkerRN=0x00000000, unsigned char IsRethrow=0) Line 1264 + 0x17 bytes C++ msvcr80d.dll!FindHandler(EHExceptionRecord * pExcept=0x0012eef0, EHRegistrationNode * pRN=0x0012ff34, _CONTEXT * pContext=0x0012ef10, void * pDC=0x0012eec4, const _s_FuncInfo * pFuncInfo=0x004bd598, unsigned char recursive=0, int CatchDepth=0, EHRegistrationNode * pMarkerRN=0x00000000) Line 779 + 0x31 bytes C++ msvcr80d.dll!__InternalCxxFrameHandler(EHExceptionRecord * pExcept=0x0012eef0, EHRegistrationNode * pRN=0x0012ff34, _CONTEXT * pContext=0x0012ef10, void * pDC=0x0012eec4, const _s_FuncInfo * pFuncInfo=0x004bd598, int CatchDepth=0, EHRegistrationNode * pMarkerRN=0x00000000, unsigned char recursive=0) Line 529 + 0x25 bytes C++ msvcr80d.dll!__CxxFrameHandler3(EHExceptionRecord * pExcept=0x0012ff34, EHRegistrationNode * pRN=0x0012ef10, void * pContext=0x0012eec4, void * pDC=0x0012ff34) Line 365 + 0x1f bytes C++ ntdll.dll!7c9037bf() ntdll.dll!7c90378b() ntdll.dll!7c937860() ntdll.dll!7c926abe() ntdll.dll!7c96cde9() ntdll.dll!7c90eafa() kernel32.dll!7c812a5b() kernel32.dll!7c812a5b() ntdll.dll!7c90e9ab() kernel32.dll!7c809524() ntdll.dll!7c90e9ab() kernel32.dll!7c8094e2() ntdll.dll!7c926abe() ntdll.dll!7c9268ad() ntdll.dll!7c91056d() dsound.dll!73f124df() dsound.dll!73f124c1() dsound.dll!73f1ea5b() dsound.dll!73f1e9f9() ntdll.dll!7c90e9ab() kernel32.dll!7c812a5b() msvcr80d.dll!_CxxThrowException(void * pExceptionObject=0x0012f2a4, const _s__ThrowInfo * pThrowInfo=0x004bc014) Line 166 C++ Earth.exe!Renderer::Blit_Surface(SDL_Rect position={...}, SDL_Surface * surface=0x01216448, SDL_Rect * clip=0x00000000) Line 159 + 0x92 bytes C++ Earth.exe!Surface::Draw(Renderer & renderer={...}, SDL_Rect position={...}, SDL_Rect * clip=0x00000000) Line 28 C++ Earth.exe!XImage::Draw(Renderer & renderer={...}) Line 57 C++ Earth.exe!XScreen<Type_List<XImage,Type_List<XButton,Type_List<XSlider,Type_List<XDialog_Box,Null_Type> > > > >::Draw(Renderer & renderer={...}) Line 85 + 0x29 bytes C++ Earth.exe!Earth::Run() Line 40 + 0x22 bytes C++ Earth.exe!SDL_main(int argc=1, char * * args=0x00b35da0) Line 29 C++ Earth.exe!_main() + 0xd1 bytes C Earth.exe!__tmainCRTStartup() Line 586 + 0x19 bytes C Earth.exe!mainCRTStartup() Line 403 C kernel32.dll!7c816fd7()
I see that the first exception thrown was here:
void Renderer::Blit_Surface(
SDL_Rect position, SDL_Surface* surface, SDL_Rect* clip)
{
	std::cout << "Last error was: " << SDL_GetError() << std::endl; // debugging only
	bool successful = (SDL_BlitSurface(surface, clip, screen, &position) == 0);
	if(!successful)
	{ // Unsuccessful blit.
		throw std::runtime_error(std::string(
			"Surface failed to blit.\nSDL: ") + SDL_GetError());
	}
}


Then, looking at that call stack again, it seems like everything starts to destruct but the Surface destructor is screwing up... Can anyone see what is wrong with this? Cheers.

Share this post


Link to post
Share on other sites
Gage64    1235
I don't see a Surface object being created anywhere. Can you post more code, including the line that triggers the exception?

Share this post


Link to post
Share on other sites
Mybowlcut    176
Sorry. I lost my internet connection for weeks and forgot about this question.

I think it's still the same issue... I'm getting:
Quote:
Unhandled exception at 0x7c812a5b in SDL_Testing.exe: Microsoft C++ exception: SDL_Surface_Exception at memory location 0x0012f9dc..

I looked at the string that SDL_GetError returned and it said:
Quote:
"Surfaces must not be locked during blit"
void Renderer::Blit_Surface(
SDL_Rect position, SDL_Surface* surface, SDL_Rect* clip)
{
bool successful = (SDL_BlitSurface(surface, clip, screen, &position) == 0);
const char* t = SDL_GetError(); // testing
if(!successful)
{ // Unsuccessful blit.
throw SDL_Surface_Exception(std::string(
"Surface failed to blit.\nSDL: ") + SDL_GetError()); // debugger hits here
}
}


I looked at the surface and it's screwed... why would all the members be so weird?


This is the line that calls the function:
s.Draw(renderer, SDL_Tools::rect(renderer.Screen_Width()-200, renderer.Screen_Height()-200,0,0), NULL);

Share this post


Link to post
Share on other sites
Mybowlcut    176
Quote:
Original post by Gage64
Any chance you're locking the surface you're blitting, or the screen surface?
Nope. I do this:
typedef std::vector<XImage> image_vec;
image_vec v(IO::read_data<XImage, image_vec>(file));
read_data just returns a container of objects as specified from the file... The problem is there. Once the image is constructed and read_data finishes, I look at the contents of v and it has the image but the internals of the surface are all messed up... could this be to do with my Surface class?

// Wrapper class for SDL_Surface that provides Copy-on-write semantics.
class Surface
{
public:
Surface() {}
Surface(const std::string& file_name, SDL_Colour* colour_key = NULL);

Surface& operator=(const Surface& rhs);

void Draw(Renderer& renderer, SDL_Rect position, SDL_Rect* clip = NULL);

Uint16 Width() const;
Uint16 Height() const;
private:
class Inner_Surface : public boost::noncopyable
{
public:
static int x;
Inner_Surface() : surface(NULL) {}
Inner_Surface(SDL_Surface* surface) : surface(surface) {}
~Inner_Surface() { SDL_FreeSurface(surface); }

SDL_Surface* surface;
};

boost::shared_ptr<Inner_Surface> inner;
};

int Surface::Inner_Surface::x = 0;
Surface::Surface(const std::string& file_name, SDL_Colour* colour_key)
:
inner(new Inner_Surface(Surface_Cache::Load_Surface(file_name, colour_key)))
{
}

Surface& Surface::operator=(const Surface& rhs)
{
inner = rhs.inner;
return *this;
}

void Surface::Draw(Renderer& renderer,
SDL_Rect position, SDL_Rect* clip)
{
renderer.Blit_Surface(position, inner.get()->surface, clip);
}

Uint16 Surface::Width() const
{
return inner.get()->surface->w;
}

Uint16 Surface::Height() const
{
return inner.get()->surface->h;
}

Share this post


Link to post
Share on other sites
Gage64    1235
Well, then post the code for read_data.

Some other points:

1) The default constructor for Surface doesn't do anything. Maybe you should remove it?

2) The other constructor can be called with one argument, so it defines an implicit conversion between an std::string and a Surface. You probably don't want this, so mark it as explicit. Same goes for Inner_Surface.

3) Instead of: inner.get()->whatever, you can just do: inner->whatever (I think).

Share this post


Link to post
Share on other sites
Mybowlcut    176
Quote:
Original post by Gage64
Well, then post the code for read_data.

Some other points:

1) The default constructor for Surface doesn't do anything. Maybe you should remove it?

2) The other constructor can be called with one argument, so it defines an implicit conversion between an std::string and a Surface. You probably don't want this, so mark it as explicit. Same goes for Inner_Surface.

3) Instead of: inner.get()->whatever, you can just do: inner->whatever (I think).
// Reads data from a file.
// - Requires that Object has an overloaded >> operator.
// - Returns a Container of Objects.
// - Returns early if file is empty.
template <typename Object, typename Container>
Container read_data(std::istream& file)
{
Container v;

if(get_file_size(file) == 0)
{ // Empty; return early.
return v;
}

std::copy(
std::istream_iterator<Object>(file),
std::istream_iterator<Object>(),
std::back_inserter(v));

return v;
}


1) I need it otherwise I'll get this error in XImage's default constructor:
Quote:
error C2512: 'Surface' : no appropriate default constructor available c:\...\ximage.cpp


2 & 3) Cheers for that. :)

Share this post


Link to post
Share on other sites
Gage64    1235
Quote:
Original post by Mybowlcut
1) I need it otherwise I'll get this error in XImage's default constructor:


This means that XImage's default constructor calls Surface's default constructor. Is this what you want?

What does XImage's default constructor looks like? And what does it's operator>>() looks like?

BTW, you don't need to check if the file is empty. If it is, std::copy() won't do anything. Also, get_file_size() might be messing up the file somehow. Try removing it and see if that makes a difference.

Share this post


Link to post
Share on other sites
Mybowlcut    176
Quote:
Original post by Gage64
Quote:
Original post by Mybowlcut
1) I need it otherwise I'll get this error in XImage's default constructor:


This means that XImage's default constructor calls Surface's default constructor. Is this what you want?

What does XImage's default constructor looks like? And what does it's operator>>() looks like?

BTW, you don't need to check if the file is empty. If it is, std::copy() won't do anything. Also, get_file_size() might be messing up the file somehow. Try removing it and see if that makes a difference.
XImage's default constructor is just empty. It calls Surface's default constructor because it's the default constructor - I don't get why this is an issue.

As for the operator>>, I've been using this:
std::istream& XImage::Read(std::istream& stream)
{
std::string buffer;
int r = 0, g = 0, b = 0;

stream >> buffer; // <
IO::Check_Opening_Tag(buffer);

stream >> name;
stream >> file_name;
stream >> position.x;
stream >> position.y;
stream >> is_colour_key;

if(is_colour_key)
{ // There is a colour key so read values.
stream >> r >> g >> b;
colour_key.r = r;
colour_key.g = g;
colour_key.b = b;
}

stream >> buffer; // >
IO::Check_Closing_Tag(buffer);
// Load surface.
Load();
return stream;
}


It was suggested to me on either this forum or CodeGuru. I can't remember why I'm using instead of directly using >> but I'm pretty sure this isn't causing the issue. The base class defines the operator>> function which derived class' Read function uses:
std::istream& operator>>(std::istream& stream, Base_XImage& image)
{
image.Read(stream);
return stream;
}




I took the file size check out of read_data but I'm still getting the error.

Share this post


Link to post
Share on other sites
Gage64    1235
Quote:
Original post by Mybowlcut
XImage's default constructor is just empty. It calls Surface's default constructor because it's the default constructor - I don't get why this is an issue.


It's not necessarily an issue. It's just that when you have a constructor that does nothing, you are enabling the creation of the object with an undefined state (i.e., the object's member variables contain garbage values).

Often people just stick an empty default constructor for their classes out of convenience. I just wanted you to think if you really need it.

Anyway, you say that after you call read_data(), the vector of images contains garbage values. Did you step through that function with the debugger? (this is actually the first thing I should have asked)

EDIT:

Quote:
read_data just returns a container of objects as specified from the file... The problem is there. Once the image is constructed and read_data finishes, I look at the contents of v and it has the image but the internals of the surface are all messed up


Actually it sounds like you did this already.

One other thing - does XImage has a proper copy constructor?

Share this post


Link to post
Share on other sites
Mybowlcut    176
Quote:
Original post by Gage64
Quote:
Original post by Mybowlcut
XImage's default constructor is just empty. It calls Surface's default constructor because it's the default constructor - I don't get why this is an issue.


It's not necessarily an issue. It's just that when you have a constructor that does nothing, you are enabling the creation of the object with an undefined state (i.e., the object's member variables contain garbage values).

Often people just stick an empty default constructor for their classes out of convenience. I just wanted you to think if you really need it.

Anyway, you say that after you call read_data(), the vector of images contains garbage values. Did you step through that function with the debugger? (this is actually the first thing I should have asked)

EDIT:

Quote:
read_data just returns a container of objects as specified from the file... The problem is there. Once the image is constructed and read_data finishes, I look at the contents of v and it has the image but the internals of the surface are all messed up


Actually it sounds like you did this already.

One other thing - does XImage has a proper copy constructor?
Cheers for replying this far. :)

I remember reading somewhere that using the operator= function as a lazy replacement for a copy constructor is bad practice... is that right?
XImage::XImage(const XImage& rhs)
{
*this = rhs;
}

XImage& XImage::operator=(const XImage& rhs)
{
if(this == &rhs) return *this;

this->name = rhs.name;
this->file_name = rhs.file_name;
this->surface = rhs.surface;
this->position = rhs.position;
this->is_colour_key = rhs.is_colour_key;
this->colour_key = rhs.colour_key;

return *this;
}

I debugged it again... and it seems that the surface is invalidated after the call to std::copy in read_data:
template <typename Object, typename Container>
Container read_data(std::istream& file)
{
Container v;

//if(get_file_size(file) == 0)
//{ // Empty; return early.
// return v;
//}

std::copy(
std::istream_iterator<Object>(file),
std::istream_iterator<Object>(),
std::back_inserter(v));

return v; // here
}



Ah... I tried some more testing:
Surface s1("3.png");

{
Surface s2("3.png");
}

return 0;
s1's surface screws up after that block executes... so I have issues somewhere in my Surface class?

[Edited by - Mybowlcut on September 2, 2008 5:50:55 AM]

Share this post


Link to post
Share on other sites
Gage64    1235
Quote:
s1's surface screws up after that block executes... so I have issues somewhere in my Surface class?


It looks like the reference-counting isn't working properly. Can you post the code for Surface's second constructor?

Also, did you try stepping through this code snippet?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this