Crash in OgreXMLConverter 1.11.3

Problems building or running the engine, queries about how to use features etc.
Post Reply
glennr
Greenskin
Posts: 126
Joined: Thu Jun 05, 2008 3:26 am
Location: Thames, New Zealand
x 9

Crash in OgreXMLConverter 1.11.3

Post by glennr »

Ogre Version: 1.11.3
Operating System: Win10
Render System: na

I just updated to 1.11.3 from 1.10.8 and I'm seeing this crash in OgreXMLConverter when converting a fairly simple .xml to .mesh.
e.g.

Code: Select all

 OgreXMLConverter mymesh.mesh.xml mymesh.mesh

Code: Select all

 	OgreMain_d.dll!Ogre::HardwareBufferManagerBase::_notifyVertexBufferDestroyed(Ogre::HardwareVertexBuffer * buf) Line 392	C++
 	OgreMain_d.dll!Ogre::HardwareVertexBuffer::~HardwareVertexBuffer() Line 61	C++
 	OgreMain_d.dll!Ogre::DefaultHardwareVertexBuffer::~DefaultHardwareVertexBuffer() Line 52	C++
 	[External Code]	
 	OgreMain_d.dll!Ogre::VertexBufferBinding::unsetAllBindings() Line 777	C++
 	OgreMain_d.dll!Ogre::VertexBufferBinding::~VertexBufferBinding() Line 751	C++
 	[External Code]	
 	OgreMain_d.dll!Ogre::HardwareBufferManagerBase::destroyVertexBufferBindingImpl(Ogre::VertexBufferBinding * binding) Line 126	C++
 	OgreMain_d.dll!Ogre::HardwareBufferManagerBase::destroyAllBindings() Line 147	C++
 	OgreMain_d.dll!Ogre::HardwareBufferManagerBase::~HardwareBufferManagerBase() Line 73	C++
 	OgreMain_d.dll!Ogre::HardwareBufferManager::~HardwareBufferManager() Line 50	C++
 	[External Code]	
 	OgreXMLConverter_d.exe!main(int numargs, char * * args) Line 602	C++
 	[External Code]	

The crash is caused by ~HardwareVertexBuffer attempting to access an instance of HardwareBufferManagerBase through a pointer that has already been deleted. The mesh does get successfully converted and the crash happens as it cleans up.

Is this a known issue and is there a fix?
Last edited by glennr on Thu Nov 08, 2018 6:58 pm, edited 1 time in total.
paroj
OGRE Team Member
OGRE Team Member
Posts: 1994
Joined: Sun Mar 30, 2014 2:51 pm
x 1074
Contact:

Re: Crash in OgreXMLConverter 1.11.3

Post by paroj »

no, this is a new issue.

However I cannot reproduce this crash here (g++7). Also it seems that the codepath is valid, as the this pointer in ~HardwareBufferManagerBase is still valid and thus the _notifyVertexBufferDestroyed call should succeed.
glennr
Greenskin
Posts: 126
Joined: Thu Jun 05, 2008 3:26 am
Location: Thames, New Zealand
x 9

Re: Crash in OgreXMLConverter 1.11.3

Post by glennr »

This has been compiled with VS2017.

The pointer that the HardwareVertexBuffer has for HardwareBufferManagerBase is not the same as the one in the call stack.
glennr
Greenskin
Posts: 126
Joined: Thu Jun 05, 2008 3:26 am
Location: Thames, New Zealand
x 9

Re: Crash in OgreXMLConverter 1.11.3

Post by glennr »

A quick'n'dirty workaround for this is to comment out the

Code: Select all

delete bufferManager
line near the end of main()

Code: Select all

    ...
    delete matMgr;
    delete meshMgr;
    //delete bufferManager;
    delete lodMgr;
    delete mth;
    ...
loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Crash in OgreXMLConverter 1.11.3

Post by loath »

i also see the access violation in 1.11.15.

short explanation:
bufferManager cleans up it's mImpl child member BEFORE cleaning up vertex buffers. next, bufferManager releases the vertex buffers which try to notify the dead mImpl causing an ACCESS VIOLATION.

Code: Select all

delete bufferManager
long explanation:
1. the bufferManager* derives from *HardwareBufferManagerBase* which is used to track the vertex, index, declaration, and binding lists. the creation of any of these types are stored in the singleton class's HardwareBufferManagerBase.

2. a hardware vertex buffer is created by OgreXmlConverter::main()->XMLToBinary()->ImportMesh()->readGeometry()->createVertexBuffer() through the bufferManager's singleton reference. a pointer to mImpl (not bufferManager) is passed into the vertex buffer.

3. "delete bufferManager" is executed and the following occurs:

a. bufferManager* releases mImpl's unique_ptr. deleting the object frees mImpl's *HardwareBufferManagerBase* data structures which are empty because callers use the bufferManager* version via the singleton.

b. next, the bufferManager* cleans up it's own *HardwareBufferManagerBase* which contains all of the actual data objects mentioned above. when it cleans up the vertex buffers, the vertex buffer tries to notify mImpl that it's been released.
this occurs in bufferManager->~HardwareBufferManagerBase()->destroyAllBindings() via:

Code: Select all

 HardwareVertexBuffer::~HardwareVertexBuffer()
 {
        if (mMgr)
        {
            mMgr->_notifyVertexBufferDestroyed(this);
        }
 }
c. mMgr is a pointer to the now deleted mImpl but since it's non-null-garbage it tries to execute the _notifyVertexBufferDestroyed(this) call on a freed invalid memory reference which causes an ACCESS VIOLATION.

i'm hoping this rings a bell and paroj knows what the fix is. :) i'm afraid i don't understand why the mImpl is split out from the main class and why they seem to share responsibilities. i also have not looked to see why this does not occur in regular ogre outside of the app. (i assume because there is a render system)

for example, if you removed the forwarding to mImpl i believe this crash would go away but i assume mImpl serves some other purpose.

thanks!
loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Crash in OgreXMLConverter 1.11.3

Post by loath »

ok i think the solution is actually pretty simple. essentially, we need to make the DefaultHardwareBufferManager class look more like the "real" hardware buffer managers for dx and gl.

1. delete the DefaultHardwareBufferManager class as it is today.
2. rename DefaultHardwareBufferManagerBase class to DefaultHardwareBufferManager.
3. the new DefaultHardwareBufferManager should derive from HardwareBufferManager instead of HardwareBufferManagerBase.
4. in the cpp file, rename all members referring to DefaultHardwareBufferManagerBase to just DefaultHardwareBufferManager.

essentially just merge DefaultHardwareBufferManager and DefaultHardwareBufferManagerBase and delete the mImpl.

i will test this out and submit a patch unless paroj sees this and disagrees with this approach.

thanks!
paroj
OGRE Team Member
OGRE Team Member
Posts: 1994
Joined: Sun Mar 30, 2014 2:51 pm
x 1074
Contact:

Re: Crash in OgreXMLConverter 1.11.3

Post by paroj »

loath wrote: Wed Sep 04, 2019 5:10 am essentially just merge DefaultHardwareBufferManager and DefaultHardwareBufferManagerBase and delete the mImpl.
this split is on purpose. DefaultHardwareBufferManager instantiates the Singleton, whereas DefaultHardwareBufferManagerBase, does not. This allows a render-system to instantiate the Singleton, while allowing additional software buffers through manually instantiating DefaultHardwareBufferManagerBase.
loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Crash in OgreXMLConverter 1.11.3

Post by loath »

i can confirm removing mImpl and collapsing the two fixes the crash.

the problem with mImpl is that the parent isn't forwarding the non-virtual functions from HardwareBufferManagerBase. so while createVertexBuffer() *IS* virtual the following are not:

createVertexDeclaration(), createVertexBufferBinding() etc... and these are going to the parent, not mImpl causing the crash.

PS. this is a race condition but i see it every time in the debugger. the reason this is painful for me is i can't bulk export from blender.
Last edited by loath on Wed Sep 04, 2019 7:07 pm, edited 1 time in total.
paroj
OGRE Team Member
OGRE Team Member
Posts: 1994
Joined: Sun Mar 30, 2014 2:51 pm
x 1074
Contact:

Re: Crash in OgreXMLConverter 1.11.3

Post by paroj »

loath wrote: Mon Sep 02, 2019 10:30 pm c. mMgr is a pointer to the now deleted mImpl but since it's non-null-garbage it tries to execute the _notifyVertexBufferDestroyed(this) call on a freed invalid memory reference which causes an ACCESS VIOLATION.
does this help?
https://github.com/OGRECave/ogre/pull/1255
loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Crash in OgreXMLConverter 1.11.3

Post by loath »

yes, that will fix it.

it seems odd there may be cases where callers can get one of two different instances of HardwareBufferManagerBase but if this meets tooling needs it's fine by me. :)

thanks!
paroj
OGRE Team Member
OGRE Team Member
Posts: 1994
Joined: Sun Mar 30, 2014 2:51 pm
x 1074
Contact:

Re: Crash in OgreXMLConverter 1.11.3

Post by paroj »

loath wrote: Wed Sep 04, 2019 7:19 pm yes, that will fix it.
great!

see this for justification: https://github.com/OGRECave/ogre/commit ... 4a67b7f40e
loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Crash in OgreXMLConverter 1.11.3

Post by loath »

awesome, thanks for the fix and history.
Post Reply