In fact, this solves a bunch of issues with keymaps and addons - to summarize, we have to update PointerRNA of keymapitems
when their operator gets (un)registered! Else, they use some freeded mem, which may crash randomly.
Details
Details
- Reviewers
Bastien Montagne (mont29) - Maniphest Tasks
- T38042: Blender crashes when I press F8
Diff Detail
Diff Detail
- Branch
- fix_f8_crash
Event Timeline
| source/blender/windowmanager/WM_api.h | ||
|---|---|---|
| 219 | Id prefer wm not be passed to all these functions, if its needed G.main->wm.first can be used internally | |
| source/blender/windowmanager/intern/wm_keymap.c | ||
| 130 | MEM_SAFE_FREE can replace this. | |
| 1102 | *picky* prefer STREQ macro. | |
| source/blender/windowmanager/intern/wm_operators.c | ||
| 191 | Why is this needed on adding? (removing makes sense), could this be made to run only when this re-registers and existing operator, rather then happening all the time? Same goes for adding macros. | |
Comment Actions
- Remove systematic keymapitems update when adding a new op or macro, rather do that for addons only in addon_utils.
Comment Actions
While updates are good, still not sure of this.
Main issue I have (IIRC) is that this patch allows registering a key before an operator is registered (WM_operator_properties_alloc_ex extra argument).
I don't think we can properly support it. Type checking wont work, and if an operator has some nested data (like collection for eg), this also will give problems.
regarding _bpy_addon_classes, Id rather this stuff not be automatic. As with class registration, it should be explicit, but in the same way we have register_module, unregister_module - its possible there could be some utility function to manage this too (I have a preference for opt-in-automagic when it comes to python internals).
I'd like this patch to be split up, first - simply fix the bug - if you remove an operator that could be used by a keymap - that shouldnt crash of course.
If you think this cant be easily split into a smaller patch then Id like to spend time to fully understand whats going on.
Having F8 work seems like we can do it without too much trouble, (onload all addons, and reload) since addons can manage own keys.
The one thing Im not sure of is having user defined keymaps, then unload addons, then load again, this is pretty confusing and not sure how to support.
Comment Actions
Remove py/addons part of the patch (will be moved to a separate one). Now this one only fixes the F8 crash.
Note that with this current code, all shortcuts defined on py operators will be lost when hitting F8
(appart for those handled by addons, which should be able to reset them), until an op is unregistered again.
Also, about the new const bool quiet arg of WM_operator_properties_alloc_ex & co, it does not allow
to register shortcuts for unexisting operators, as this is already possible in current master. It just
allows to do it with getting the noisy warning that op "foobar" wasn’t found, in console. This is needed
to avoid hundreds of such lines each time you hit F8!