View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005508||DarkRadiant||Saving and loading||public||25.01.2021 20:42||29.01.2021 04:03|
|Target Version||2.11.0||Fixed in Version||2.11.0|
|Summary||0005508: Crash in master when loading a map|
|Description||This one has been there since at least 0005493 was fixed in commit 40e534bf74b0b5f1decc012f49db332c0751ce79. It may have been introduced earlier but wasn't getting hit due to the previous crash.|
This happens when loading a more fully featured map, but not when loading any of my dedicated map files that just contain my modular components. A brand new map with nothing in it won't crash either. I'm still in the process of adding things one-by-one until I create a map that crashes, but figured I'd post the bug now in case the location and stack trace are more familiar to you.
The crash happens immediately after the loading is complete and the map gets rendered to the window for the first time.
|Steps To Reproduce||1. Open DarkRadiant.|
2. Open a fully-featured map. My own caused a crash, and I also confirmed the crash with Perilous Refuge (peril2).
|Additional Information||Screenshot with debug information is attached, "owningNode" appears to be invalid. Stack trace isn't super helpful, it's all WX so the actual caller probably posted an event.|
Let me know if you'd like me to help narrow down which map feature is causing the crash.
|Tags||No tags attached.|
Screenshot_20210125_150817.png (696,636 bytes)
|P.S the latest master currently needs the usual round of trivial Linux fixes for a couple missing includes, but didn't want to toss it into the middle of what you were working on :-)|
|Hm, this stracktrace looks familiar, I think already fixed the same or very similar issue. It's caused by the optimistic approach of the wxGTK implementation posting idle events to expand ancestors, ignoring the possibility that the wxDataViewItem might actually be invalid by the time the idle event kicks in. There's no way to clear that event or clear the item unless somebody calls EnsureVisible(wxDataViewItem(nullptr)) after changing the model.|
I've done some more digging and I believe you are correct. I'm 100% certain I'm still experiencing 0005498, even after your fix was applied. I can prevent the crash by removing the "LastShaderClipboardMaterial" key/value from the map.darkradiant file.
I see your `EnsureVisible(wxDataViewItem(nullptr))` that you added to mitigate this issue getting called early on, but if I put a breakpoint on that line and also on both `EnsureVisible(item)` lines in ResourceTreeView.cpp, I see the the latter getting called last, after the map is loaded and rendered but before the crash. I assume this would lead to that idle event getting reactivated and causing the crash.
If you can't reproduce 0005498 in the latest master when opening a map that contains a "LastShaderClipboardMaterial", let me know and I can send you the stack trace showing where `EnsureVisible(item)` is getting called.
There must be a code path I missed when placing that fix calling EnsureVisible(null). If ResourceTreeView.cpp:EnsureVisible is called last, there must be something happening in between that removes the model from the view (or clears the items). Can you maybe figure out if the AssociateModel() method is invoked after the item is scheduled by EnsureVisible(item)?
Thing is, the MediaBrowser is reloaded once a map is finished loading. The ShaderClipboard notifier (LastShaderClipboardMaterial) schedules an item selection in the MediaBrowser, which is cleared almost immediately afterwards by the repopulator (at least I think that's the trigger) and the scheduled item is no longer valid. We need to figure out why the fix in AssociateModel is not enough in this case.
If I load a map without the LastShaderClipboardMaterial, I do see AssociateModel() getting called, presumably as you described. However, it happens as a result of ResourceTreeView::_onTreeStorePopulationFinished and that is a callback for a WX event. I imagine the the expand ancestors event is somehow getting scheduled ahead of the population finished event.
I'll keep playing with this and let you know what else I find.
I found the reason - it's the Clear() call that is happening before the populator thread is starting. I'll have to think about how to rearrange that whole thing to make it safe.
To be honest, I don't like the wxGTK implementation of scheduling an idle event with a dataviewitem that might be out of date by the time it's processed. There's no wxDataViewModelNotifier listening for item removed events to cancel the callback if the item is stale.
The real solution would be to not embed the actual pointers to our Node structures in the wxDataViewItem, since there's no way to know whether the pointer is valid or stale. Some other ID that can be checked for validity would be nice, but this is of course some more work. The cheap solution of placing an EnsureVisible(null) in ResourceTreeView::Clear() would fix this particular crash, but it's not even close to be waterproof.
|25.01.2021 20:42||jonri||New Issue|
|25.01.2021 20:42||jonri||File Added: Screenshot_20210125_150817.png|
|25.01.2021 20:58||jonri||Note Added: 0013537|
|26.01.2021 03:19||greebo||Note Added: 0013538|
|26.01.2021 03:19||greebo||Status||new => acknowledged|
|26.01.2021 23:30||jonri||Note Added: 0013550|
|28.01.2021 18:07||greebo||Status||acknowledged => feedback|
|28.01.2021 18:07||greebo||Note Added: 0013555|
|28.01.2021 18:07||greebo||Note Edited: 0013555||View Revisions|
|28.01.2021 18:33||jonri||Note Added: 0013556|
|28.01.2021 18:33||jonri||Status||feedback => new|
|28.01.2021 18:58||greebo||Note Added: 0013557|
|28.01.2021 18:59||greebo||Note Edited: 0013557||View Revisions|
|28.01.2021 19:04||greebo||Assigned To||=> greebo|
|28.01.2021 19:04||greebo||Status||new => assigned|
|29.01.2021 03:54||greebo||Relationship added||related to 0005513|
|29.01.2021 04:03||greebo||Changeset attached||=> DarkRadiant master 8024e813|
|29.01.2021 04:03||greebo||Target Version||=> 2.11.0|
|29.01.2021 04:03||greebo||Status||assigned => resolved|
|29.01.2021 04:03||greebo||Resolution||open => fixed|
|29.01.2021 04:03||greebo||Fixed in Version||=> 2.11.0|