Page 1 of 1

Crash in OgreXMLConverter 1.11.3

Posted: Thu Nov 08, 2018 12:19 am
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?

Re: Crash in OgreXMLConverter 1.11.3

Posted: Thu Nov 08, 2018 1:38 pm
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.

Re: Crash in OgreXMLConverter 1.11.3

Posted: Thu Nov 08, 2018 7:01 pm
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.

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Feb 13, 2019 3:46 am
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;
    ...

Re: Crash in OgreXMLConverter 1.11.3

Posted: Mon Sep 02, 2019 10:30 pm
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!

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 5:10 am
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!

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 6:56 pm
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.

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 7:05 pm
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.

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 7:05 pm
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

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 7:19 pm
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!

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 8:19 pm
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

Re: Crash in OgreXMLConverter 1.11.3

Posted: Wed Sep 04, 2019 10:11 pm
by loath
awesome, thanks for the fix and history.