allocation/deallocation issues [general C++/vectors]

[Cube_]

Yes thanh, I know vectors are dynamic.
However if I do not pre-allocate them it's not entirely impossible that they may need re-allocating at a later point (further I'm not to keen on using an object to store a bunch of objects, that's a bit too meta for me).

Anyway, I'll keep tinkering with this.
Eventually I'll figure out why it crashes or at least figure out more precisely where it crashes (which will maybe help me track down why)

[Cube_]

Okay, back to my theory that the irrlicht.dll in the irrlicht archive being incompatible with mingw-gcc-4.8.x


Either that or I'm sending some *weird* data to the irrlichtDevice.
That is however unlikely(?) given that it happens on the second runthrough, if it was weird data surely it'd be after the first run-through since the exact same sequence runs on completely cleared lists that have then been repopulated to their state prior to initial building (i.e a list of nodes and chunk metadata containing 8 entries of NULL each and a list of 8 booleans all set to true).
It is inconclusive however as the irrlicht.dll in use does not have debugging symbols which completely stops gdb from being able to continue running the program line by line ("Cannot find bounds of current function").

[Cube_]

that's odd, I don't even call the OnAnimate method
I don't understand this error but from what I can tell it crashes after the drawall call (when stepping through line by line I can see the new chunks meaning it renders... then my program dies)

[CuteAlien]

OnAnimate is called in drawAll(). Most common reason for a crash is likely that your pointer is invalid (pointing to something else or released already). The place where it crashes also hints at that - as it's most likely caused by 'Children' in ISceneNode being corrupt. Which is unlikely to happen unless you work with a corrupt pointer.

Can't tell you how to fix that as there can be an infinte number of reasons how it got like that. At some point your application you mess with your pointers.

[Cube_]

yes, I figured as much.
However to track it down will be hard, unless maybe I decide to add a breakpoint everyime my nodes list is read, written or otherwise used.
That way I can tell where it goes awry (in theory).

Is there any way to handle this error gracefully instead of crashing? If a node is missing/corrupt/whatever I'd like for it to at least be able to print useful diagnostic messages before crashing (could I catch this as an exception?)

[CuteAlien]

Pff... crash is a graceful thing to do when you have memory corruptions. And yeah - there are ways to catch this. But use google for this.

[thanhle]

If you are deleting or add scenode on a different thread or between draw call then you will have error.
Make sure the delete or add node is done before begin or after endScene.

Also I think you need a vector of objects. This way you don't need to manage multiple deletes, which can be tedious if you have too many different type of vectors of various thing.


Regards,
thanh

[Cube_]

damn, debugging memory corruption is proving hard.
Static code analysis ended up being more or less useful unless

Code: Select all

'class chunk' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory

.
means anything (it contains the node pointer until it scopes out but it also sends the node pointer to the node vector).

Thanh, I don't delete or add scene nodes between begin and endscene, (I do so after endscene, not that it makes a difference. It crashes at the same point either way).

As for having too many lists, I have two.
Two isn't too many methinks, further I rather like having two. There are various reasons as to why I would like to delete them asynchronously at a later point (keeping the metadata of chunks that are occluded in memory so I can generate meshes for them when needed, so I can delete meshes from said chunks if they become occluded etc).
A vector of objects makes this impossible (in a clean manner).

[Cube_]

Anyway, from what I have narrowed it down to the error must be in the delete function, I've changed my code so that the exact same steps that happen after the delete function happen before (really the only difference was to just call buildList instead of populating the build list with known values ahead of time (which was a hack I intended to do away with anyway).

The behavior is still identical.
So here's the runtime as a step list.
main
init chunkmgr
poll build list.
generate meshes
tick for 20 ticks
poll lists
delete as requested by unloadlist
generate as requested by buildlist
tick at 20 seconds (crashes instead of getting here).

the unload list function is identical to the way it was in my last post about it aside from CuteAlien's optimization suggestions.
as a recap

Code: Select all

void chunkMgr::unloadList()
{
    std::vector<chunk*>::iterator it;
    std::vector<IMeshSceneNode*>::iterator it2;
    it = loadID.begin();
    it2 = nodes.begin();
    while(it2 != nodes.end())
        {
            fflush(stdout);
            printf("Deleting chunk data of chunk #%d & nodes of chunk #%d\n",*it,*it2);
            delete *it2;
            ++it2;
        }
    while(it != loadID.end())
    {
        delete *it;
        ++it;
    }
        fflush(stdout);
        printf("Clearing Lists...\n");
        nodes.clear();
        loadID.clear();
        //*
        for(int i = 0; i != indx; i++)
        {
            nodes.push_back(NULL);
            loadID.push_back(NULL);
        }//*/
}

Currently attempting to see what happens if I do the initial allocation in the main loop instead of before entering it (at 10 ticks in, before beginScene)

[Cube_]

now the output looks as such:

Code: Select all

Irrlicht Engine version 1.8.1
Microsoft Windows 7 Ultimate Edition Service Pack 1 (Build 7601)
WGL_extensions: WGL_EXT_depth_float WGL_ARB_buffer_region WGL_ARB_extensions_string WGL_ARB_make_current_read WGL_ARB_pixel_format WGL_ARB_pbuffer WGL_EXT_extensions_string WGL_EXT_swap_control WGL_ARB_pixel_format_float WGL_ARB_framebuffer_sRGB 
Pixel Format: 4
Using renderer: OpenGL 2.1.0
Intel(R) HD Graphics: Intel
OpenGL driver version is 1.2 or better.
GLSL version: 1.2
Resizing window (800 600)
Number of blocks: 31 
Loaded texture: C:/Users/shiina/Projects/projects/HW/res/texture.png
Loaded texture: C:/Users/shiina/Projects/projects/HW/res/texture1.png
bList contains 8 elements with a capacity of 32
loaded ID list contains 8 elements with a capacity of 32
tick 1
tick 2
tick 3
tick 4
tick 5
tick 6
tick 7
tick 8
tick 9
tick 10
populating build list
Queueing build of local chunk #0
Queueing build of local chunk #1
Queueing build of local chunk #2
Queueing build of local chunk #3
Queueing build of local chunk #4
Queueing build of local chunk #5
Queueing build of local chunk #6
Queueing build of local chunk #7
1
Time: 462 ms
0063dc18
Local chunk #0 built...
1
Time: 520 ms
0063dc1c
Local chunk #1 built...
1
Time: 588 ms
0063dc20
Local chunk #2 built...
1
Time: 642 ms
0063dc24
Local chunk #3 built...
1
Time: 695 ms
0063dc28
Local chunk #4 built...
1
Time: 746 ms
0063dc2c
Local chunk #5 built...
1
Time: 800 ms
0063dc30
Local chunk #6 built...
1
Time: 854 ms
0063dc34
Local chunk #7 built...
All Chunks Built.
tick 11
tick 12
tick 13
tick 14
tick 15
tick 16
tick 17
tick 18
tick 19
tick 20
Deleting chunk data of chunk #6538592 & nodes of chunk #35955168
Deleting chunk data of chunk #6538592 & nodes of chunk #81338824
Deleting chunk data of chunk #6538592 & nodes of chunk #82574408
Deleting chunk data of chunk #6538592 & nodes of chunk #82574792
Deleting chunk data of chunk #6538592 & nodes of chunk #83418704
Deleting chunk data of chunk #6538592 & nodes of chunk #83419520
Deleting chunk data of chunk #6538592 & nodes of chunk #82805832
Deleting chunk data of chunk #6538592 & nodes of chunk #96407120

This would imply that the error is still in the deletion code as it crashes right at the end of it (evidently from restructuring the calls so as to delay the generation, this makes it slightly easier to debug as it doesn't have time to start building more chunks before crashing).

Do I understand why it crashes or what the hell I'm doing?
nope

[Cube_]

I'm pretty damn sure I've figured it out (in fact, while writing this my game compiled and ran.. and ran... and ran.. .and ran so I know it works).

The new crash message from restructuring made it all so much clearer. (because it happened at the actual error site, not long after)

Output log (not interesting per se, it shows expected output as compared to a crash) http://pastebin.com/KPqpKJU3

The new unloading function (I'll have to figure out how to use iterators instead of an int counter but time for celebration.
this has haunted me for much too long)

Code: Select all

void chunkMgr::unloadList()
{
    std::vector<chunk*>::iterator it;
    std::vector<IMeshSceneNode*>::iterator it2;
    it = loadID.begin();
    it2 = nodes.begin();
    int counter = 0;
    
    while(it != loadID.end())
    {
        delete *it;
        nodes.at(counter)->remove();
        ++it;
        ++counter;
    }
    while(it2 != nodes.end())
    {
        fflush(stdout);
        printf("Deleting chunk data of chunk #%d & nodes of chunk #%d\n",*it,*it2);
        //delete *it2;
        //it2->drop();
        ++it2;
    }
        fflush(stdout);
        printf("Clearing Lists...\n");
        nodes.clear();
        loadID.clear();
        //*
        for(int i = 0; i != indx; i++)
        {
            nodes.push_back(NULL);
            loadID.push_back(NULL);
        }//*/
}

pretty sure I just need to dereference the pointer to use it with an iterator, either way that felt good.
And after several minutes of testing I can conclude it doesn't leak either (seeing I allocate tens of megabytes of just chunk metadata, let alone mesh data, I'd notice pretty damn fast).

[CuteAlien]

Ah yes, that's the kind of stuff we easily miss when not seeing the full code. Your main difference here is _not_ that you rewrote the loop. It's that you do remove() on IMeshSceneNode* instead of a delete. Which certainly makes sense with Irrlicht. At least maybe. The cleaner solution might be to have additional grab() and drop() calls (reason: remove() is about removing it frome the scenemanager/scenegraph. But as soon as you keep own pointers to Irrlicht objects you should grab() them for as long as you need them and remove() them at the end. But really depends on the rest of the code if that is necessary).

it2->drop() once more could not work - that's because you try to drop the iterator object instead of it's content. What you wanted was (*it2)->drop(). Or in this case (*it2)->remove(). Then you don't need the counter.

And one more change - if you work with function calls (like drop and remove) _and_ you have an array which can contain null-pointers then you should really do: if (*it) (*it2)->remove();
Otherwise it'll crash sooner or later because you do a function call on a null-pointer.

And last thing - if you plan to fill it again with NULL directly afterwards, then don't clear() at all. You you can do a "*it = NULL;" after remove() or delete calls and set the content directly to 0.

[thanhle]

You should be aware that Irrlicht is not thread safe.

You can't delete asynchronously. Unless you setting up some delete flag inside the rendering loop.

From my experience. The best way to do that is add delete items to a delete list. Then inside the rendering loop doing the actual delete somewhere in there. I can delete > 5000 scenodes that way without corruption.
I do the same thing for adding scenodes dynamically.

Apart from delete or add scenode. You can do whatever to the node asynchonously.

Regards,
thanh

[thanhle]

This is one way of doing it.

for (int i = (lstRemoveElements.size() - 1); i >= 0; i--)
{
Element *removeItem = lstRemoveElements;
removeItem->getSceneNode()->remove(); //This remove the scenenode. Irrlicht will automatically delete this. We don't need to delete this pointer.
lstRemoveElements.erase(lstRemoveElements.begin() + (i));
}

I know it is slow. You can do the clear(). But it is the way I needed.
Regards,
thanh

[Cube_]

got it (*it2)-remove();
I figured it was something like that, I tried *it2->remove, that apparently didn't work (I assume the arrow operator has higher precedence)

As for the array being able to contain null pointers, while that is true I can't see how a null pointer would ever cause an issue.
the unload list is always populated to the amount I wish to remove (all for the test but later on it will typically contain no more than 4 chunks at any time), so this would be safe in that manner as it's only populated up to the amount of entries that are actually to be removed (I don't use null padding, I fill it with null as a placeholder but that'll be moved to the chunk selector logic to prevent possible attempts to operate on null pointers).
The reason for clearing it is of course to allow this.

thanh, I already have the facilities to use a delete list already, currently I operate on the entire list for stress testing but I'll actually run a delete list to delete as needed (the same reason I have a build list, to allow building asynchronously instead of locking the main thread... I really should figure out a way to build these in a separate thread but that's a worry for a later time).

Anyway, again I thank you for putting up with my incompetence.
I shall now implement some more logic using coding conventions I'm familiar with (thankfully I need not worry about vectors anymore)