bmx-ng / brl.mod

BlitzMax Runtime Libraries, for BlitzMax NG.
12 stars 12 forks source link

Various sigsev/segfaults with brl.mod and bcc-ng #24

Closed GWRon closed 8 years ago

GWRon commented 8 years ago

With commit a354458c1402d04a6d67b177e420167b116d1d82 I get a segfault on startup.

Program received signal SIGSEGV, Segmentation fault.
0x0869eee1 in _brl_linkedlist_TList_ObjectEnumerator (o=0x8956b78)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/linkedlist.mod/.bmx/linkedlist.bmx.debug.linux.x86.c:1721
1721        bbt_enum=((struct brl_linkedlist_TListEnum_obj*)bbObjectDowncast((brl_linkedlist_TListEnum__pool)->clas->md_RemoveFirst(brl_linkedlist_TListEnum__pool),&brl_linkedlist_TListEnum));
(gdb) bt
#0  0x0869eee1 in _brl_linkedlist_TList_ObjectEnumerator (o=0x8956b78)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/linkedlist.mod/.bmx/linkedlist.bmx.debug.linux.x86.c:1721
#1  0x085d153d in _brl_reflection_TTypeId__Resolve (o=0x890bf78)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:3756
#2  0x085b51f2 in brl_reflection_TTypeId__Update ()
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:3068
#3  0x085cf96a in brl_reflection_TTypeId_ForObject (bbt_obj=0x88d4d00)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:2511
#4  0x083525a5 in _bb_dig_base_util_helper ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.util.helper.bmx.debug.linux.x86.c:1374
#5  0x0834407f in _bb_dig_base_gfx_gui_list_base ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.gfx.gui.list.base.bmx.debug.linux.x86.c:4489
#6  0x08338dcc in _bb_dig_base_gfx_gui_list_slotlist ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.gfx.gui.list.slotlist.bmx.debug.linux.x86.c:2630
#7  0x081933bf in _bb_source_main ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/.bmx/main.bmx.debug.linux.x86.c:123822
#8  0x0804e0b8 in _bb_main ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/.bmx/TVTower.bmx.console.debug.linux.x86.c:16
#9  0x0860e32c in __bb_brl_appstub_appstub ()
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/appstub.mod/.bmx/appstub.bmx.debug.linux.x86.c:8
#10 0x0804df74 in main (argc=1, argv=0xffffd3c4) at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/appstub.mod/appstub.linux.c:18

Reverting to the one before ("[osx] Don't use deprecated Microseconds()." - 065ddd13e499d3fa9b3b5a9dfb1f8b8d036c70b9) moves the segfault to a later stage (after loading some ressources)

Program received signal SIGSEGV, Segmentation fault.
bbObjectDowncast (o=0x0, t=t@entry=0x880a2a0 <_base_gfx_gui_TGUIobject>)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/blitz.mod/blitz_object.c:97
97      BBClass *p=o->clas;

(gdb) bt
#0  bbObjectDowncast (o=0x0, t=t@entry=0x880a2a0 <_base_gfx_gui_TGUIobject>)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/blitz.mod/blitz_object.c:97
#1  0x0831e957 in __base_gfx_gui_TGUIManager_Update (o=0x8908f78, bbt_State=0x87f66c0 <_s905>, bbt_fromZ=-1000, bbt_toZ=-1000, bbt_updateTypes=3)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.gfx.gui.bmx.debug.linux.x86.c:2236
#2  0x08053170 in __main_TScreen_MainMenu_Update (o=0x89029c0, bbt_deltaTime=0,0333333351)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/.bmx/main.bmx.debug.linux.x86.c:41521
#3  0x083aff2d in __common_misc_screen_TScreenCollection_UpdateCurrent (o=0x89ae1b0, bbt_deltaTime=0,0333333351)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/.bmx/common.misc.screen.bmx.debug.linux.x86.c:902
#4  0x0818b94f in _main_TApp_Update ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/.bmx/main.bmx.debug.linux.x86.c:38331
#5  0x082f4639 in __base_util_deltatimer_TDeltaTimer_RunUpdate (o=0x8986ea0)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.util.deltatimer.bmx.debug.linux.x86.c:370
#6  0x082f4b1b in __base_util_deltatimer_TDeltaTimer_Loop (o=<optimized out>)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.util.deltatimer.bmx.debug.linux.x86.c:485
#7  0x081930fd in _main_StartTVTower (bbt_start=bbt_start@entry=1)
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/.bmx/main.bmx.debug.linux.x86.c:123766
#8  0x0804e118 in _bb_main ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/.bmx/TVTower.bmx.console.debug.linux.x86.c:27
#9  0x0860e32c in __bb_brl_appstub_appstub ()
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/appstub.mod/.bmx/appstub.bmx.debug.linux.x86.c:8
#10 0x0804df74 in main (argc=1, argv=0xffffd3c4) at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/appstub.mod/appstub.linux.c:18

Seems it is not a matter of a special module but the "downcast" way.

So I reverted "BCC" to an older revision ("Boolean expressions should autocast to String." bmx-ng/bcc.git@57588c48c9712ae0611b59e8264b9616eb9e39e0). Recompiled modules and there I got an other crash - or better - it stalled and I had to CTRL+C the execution in GDB:

^C
Program received signal SIGINT, Interrupt.
0x086a6063 in bbArrayIndex (arr=0x88d3ed8, offset=1, index=5) at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/blitz.mod/blitz_array.c:306
306     if (index < 0 || index >= arr->scales[0]) brl_blitz_ArrayBoundsError();
(gdb) bt
#0  0x086a6063 in bbArrayIndex (arr=0x88d3ed8, offset=1, index=5)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/blitz.mod/blitz_array.c:306
#1  0x0860f0f3 in brl_appstub_debugger_mt_stdio_OnDebugLeaveScope ()
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/appstub.mod/.bmx/debugger_mt.stdio.bmx.debug.linux.x86.c:1188
#2  0x0869ecea in _brl_linkedlist_TList_InsertBeforeLink (o=0x88d4cc0, bbt_value=0x884ea84 <bbNullObject>, bbt_succ=0x8913d10)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/linkedlist.mod/.bmx/linkedlist.bmx.debug.linux.x86.c:814
#3  0x0869c37a in _brl_linkedlist_TList_AddLast (o=0x88d4cc0, bbt_value=0x884ea84 <bbNullObject>)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/linkedlist.mod/.bmx/linkedlist.bmx.debug.linux.x86.c:553
#4  0x085d11c1 in _brl_reflection_TTypeId__Resolve (o=0x8901b58)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:3896
#5  0x085b5bf2 in brl_reflection_TTypeId__Update ()
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:3068
#6  0x085d036a in brl_reflection_TTypeId_ForObject (bbt_obj=0x88d4d00)
    at /home/ronny/Arbeit/Programmieren/BlitzMaxNG/mod/brl.mod/reflection.mod/.bmx/reflection.bmx.debug.linux.x86.c:2511
#7  0x08351ec5 in _bb_dig_base_util_helper ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.util.helper.bmx.debug.linux.x86.c:1374
#8  0x083437ef in _bb_dig_base_gfx_gui_list_base ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.gfx.gui.list.base.bmx.debug.linux.x86.c:4507
#9  0x083384bc in _bb_dig_base_gfx_gui_list_slotlist ()
    at /home/ronny/Arbeit/Programmieren/Projekte/Apps/TVTower.WorkingCopy/TVTower.git---NG/source/Dig/.bmx/base.gfx.gui.list.slotlist.bmx.debug.linux.x86.c:2639
#10 0x08193b9f in _bb_source_main ()

I do not know whether it is BCC related - or a mix of both. To research I would need to check each combination of "modules", "bmx" and "bcc". This is more than tedious - so I leave that open for you (who surely finds the reason way easier than I need of time to narrow the bug down to a specific revision-mix).

woollybah commented 8 years ago

Enabling DEBUG in bcc's config.bmx shows that one of the links is NULL (0)

 : 6 :: Null Pointer : o->_brl_linkedlist_tlistenum__link 

...

#3  0x08393b3a in _brl_linkedlist_TListEnum_NextObject (o=0x93ce858)
    at /home/brucey/000_programming/BlitzMaxTestArea/mod/brl.mod/linkedlist.mod/linkedlist.bmx:108
#4  0x081f1efc in __base_gfx_gui_TGUIManager_Update (o=0x86aff78, bbt_State=0x859f6e8 <_s908>, bbt_fromZ=-1000, bbt_toZ=-1000, 
    bbt_updateTypes=3) at /home/brucey/000_programming/BlitzMaxTestArea/src/TVTower_git/source/Dig/base.gfx.gui.bmx:512

This of course should not happen, and it's difficult to determine the cause of this at the moment.

woollybah commented 8 years ago

Interestingly (having added some debug to your code), you appear to be modifying ListReversed as you are iterating over it. This is probably never a good idea generally, and perhaps more so with the new GC as it may be cleaning up more quickly than the legacy one?

in TGUIManager:Update() ...

ITERATING
====== PREPARE NEW GAME ======
TGameState.Initialize(): Reinitialize all game objects
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
SORTING
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
ADDING
SORTING
REMOVING
SORTING
ADDING
SORTING
SORTING
ADDING
... and so on until it crashes.

Using a copy of ListReversed in the loop appears to fix the crashing...

GWRon commented 8 years ago

Maybe GC kicking in ? Or some kind of concurrency (change of the list in the same time) - happening because of some optimization of GCC?

Regarding the line you posted... I thought it was my mistake (created a local list backup with just assigning the original list instead of creating a copy - to avoid the list getting manipulated in the loop):

        'store a list of special elements - maybe the list gets changed
        'during update... some elements will get added/destroyed...
        'old: Local ListDraggedBackup:TList = ListDragged
        Local ListDraggedBackup:TList = ListDragged.Copy()

        'first update all dragged objects...
        if GUIMANAGER_TYPES_DRAGGED & updateTypes
            For Local obj:TGUIobject = EachIn ListDragged

... ok just before reading your new response I got aware of this too :-). Sorry for the fuss.

Edit: Whats your advice for a "performance wise" approach to handle such things? Cannot imagine that copying a list before a big "update all"-cycle is the best option. Seems BlitzMax does not have a "TConcurrentMap" or "TConcurrentList" (might come in handy :-)).

woollybah commented 8 years ago

Oh yes, I didn't notice that, but not sure what the backup variable is for? If ListDragged will be modified while you are iterating over it you should probable use ListDraggedBackup in your Eachin. ... as well as fixing the following code block for ListReversed

GWRon commented 8 years ago

The backup list was intended to hold the "original list of dragged elements" - during the update of dragged elements they might get "no longer dragged". In that case they wont be any longer in the dragged list ..

So when updating all non-dragged in the next step, the backup list was there to skip all already updated items.

Also the update of an item could have created a new dragged item - which is then normally "updated" with the rest of the non-dragged elements.

I understand that I should normally append them to an array, use some kind of while-loop (to even fetch the new ones) and update the array according to various sort options (priority, zindex...).

That whole GUI portion should be rewritten somewhen (having a closer look to how wxwidget handles things).

Regarding ListReverse: I assume I could use the "TLink" value to "reversely traverse" through the list - without needing to have a reversed List. So It only needs one "backup" before updating everything.

woollybah commented 8 years ago

Ah, I see it there in the other list now :-) Thanks.

GWRon commented 8 years ago

Hmm, you posted in that thread: http://www.blitzmax.com/Community/posts.php?topic=82916#935611

Will have a look at it.

woollybah commented 8 years ago

One other thing, in Add() you are also calling AddFirst() on ListReversed, and then sorting the list, which completely replaces ListReversed anyway. No point populating a list you are about to throw away?

GWRon commented 8 years ago

Hmm .. I think you are right :-)

Replaced the "reversed" portion with TListReversed.Create(list/backup) but this leads to a segfault right a the beginning of a release build (debug runs fine) ... investigating ... surely a null thing.

Edit I tried to use your TListReversed:

' Copyright (c) Bruce A Henderson
SuperStrict
Import brl.linkedlist

Type TListReversed Extends TList
    Field _origHead:TLink

    'override: restore original head
    Method Delete()
        _head = _origHead
    End Method

    Method Create:TListReversed(list:TList)
        _origHead = _head
        _head = list._head._pred

        Return Self
    End Method

    Method ObjectEnumerator:TListEnum()
        Local enum:TListReversedEnum = New TListReversedEnum

        enum._link = _head

        Return enum
    End Method

End Type

Type TListReversedEnum Extends TListEnum
    Method HasNext:Int()
        Return _link._value <> _link
    End Method

    Method NextObject:Object()
        Local value:Object = _link._value

        Assert(value <> _link)

        _link = _link._pred

        Return value
    End Method
End Type

I tried to use it in "updateChildren()" but it segfaults then (Aborted (core dumped))... so might the code be "outdated" ?

    Method UpdateChildren:Int()
        If Not children Then Return False

'       local childrenBackup:TList = new TListReversed.Create(children.Copy())
        local childrenBackup:TList = children.Copy().Reversed()
        'update added elements
        For Local obj:TGUIobject = EachIn childrenBackup

            'avoid getting updated multiple times
            'this can be overcome with a manual "obj.Update()"-call
            'if obj._lastUpdateTick = GUIManager._lastUpdateTick then continue
            'obj._lastUpdateTick = GUIManager._lastUpdateTick

            obj.update()
        Next
    End Method

(I know that there is no use in using TListReversed in that case - just tried it in that code portion when replacing possible "old variants" of the reversed list)

Regarding "GUI" it might be better to store the reversed list (used for updates) and only reverse the reversed list for drawing. Why? There should be no modification of the list during "draws" while a modification of the list content could happen during "update". This would save a lot of "list copying".

What do you think about adding a "locked"-flag to the lists? Is the locked flag set is each removal or adding not allowed (trow "bla") - so it would get way more clear if there is something changing the container list within the loop. Checks for "if flag" should only be done during add/remove and not while iterating, so it should not add that much compared to many "copy()" calls. Do you see that differently?

GWRon commented 8 years ago

I thought about adding some kind of "delayed transaction"-log which holds each request to "removed/added" objects during a loop over a "locked list".

So each "HasObject"-request is then checking the original list and then altering the boolean value with each "r_actioncount" or "a_actioncount"-object found in the log-map.

That "r" and "a"-entries are there to allow dynamic add and removal during a loop.

When adding to the list this has then to get added to the log instead of the actual file. Same for removal. When then unlocking the list again, the log is "processed" and the actual list is manipulated.

Similar to transactions in Databases.

BUT: isn't this a bit too much overhead compared to "listbackup:TList = list.Copy()" before a loop and traversing over the backup? I can imagine that the performance increase and memory improvement is only visible for lists with some thousand entries.

woollybah commented 8 years ago

Well, you shouldn't really modify while iterating/enumerating. Java has similar constraints, and will raise an exception if it catches you doing so.

GWRon commented 8 years ago

Have had this in JAVA ... but old code, old habits.

Regarding my idea of enhancing list/map to support some kind of "transaction". If bypassing each modification when locked (auto lock when using the enumerator) and adding the action to a log, this should would come in handy (a TConcurrencyMap / List). Just do not know how to "auto-unlock" aka recognize if an enumeration stopped. Else it needs a "Lock()" and "Unlock()" command taking care of such things (Unlock() only if the "lockedCount" is 0 again - to wait until a recursive loop over the list finishes). The helper functions ("contains()") then of course need to handle the log entries (added, removed, added -> contains = true).

Your changes to map/list will surely have a large impact on finding bugs in my code (... it will show a big pile of them :-))