From 9a031b43707c68ebe17228c37abab3320d37dc55 Mon Sep 17 00:00:00 2001 From: Myk Taylor Date: Mon, 30 Dec 2024 05:19:28 -0800 Subject: [PATCH 1/2] don't lock over INTERPOSE_NEXT since it may end up deadlocking, depending on what's in the call chain --- plugins/eventful.cpp | 84 +++++++++++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 32 deletions(-) diff --git a/plugins/eventful.cpp b/plugins/eventful.cpp index 1723c6398f..267119b92c 100644 --- a/plugins/eventful.cpp +++ b/plugins/eventful.cpp @@ -1,11 +1,9 @@ -#include "Error.h" -#include "Console.h" -#include "Export.h" #include "LuaTools.h" -#include "MiscUtils.h" #include "PluginManager.h" #include "VTableInterpose.h" +#include "modules/EventManager.h" + #include "df/building.h" #include "df/building_furnacest.h" #include "df/building_workshopst.h" @@ -23,8 +21,6 @@ #include "df/unit_wound.h" #include "df/world.h" -#include "modules/EventManager.h" - #include #include #include @@ -33,7 +29,6 @@ using std::vector; using std::string; using std::stack; using namespace DFHack; -using namespace df::enums; DFHACK_PLUGIN("eventful"); REQUIRE_GLOBAL(gps); @@ -304,13 +299,18 @@ struct workshop_hook : df::building_workshopst{ typedef df::building_workshopst interpose_base; DEFINE_VMETHOD_INTERPOSE(void,fillSidebarMenu,()) { - CoreSuspender suspend; color_ostream_proxy out(Core::getInstance().getConsole()); bool call_native=true; - onWorkshopFillSidebarMenu(out,this,&call_native); + { + CoreSuspender suspend; + onWorkshopFillSidebarMenu(out,this,&call_native); + } if(call_native) INTERPOSE_NEXT(fillSidebarMenu)(); - postWorkshopFillSidebarMenu(out,this); + { + CoreSuspender suspend; + postWorkshopFillSidebarMenu(out,this); + } } }; IMPLEMENT_VMETHOD_INTERPOSE(workshop_hook, fillSidebarMenu); @@ -319,13 +319,18 @@ struct furnace_hook : df::building_furnacest{ typedef df::building_furnacest interpose_base; DEFINE_VMETHOD_INTERPOSE(void,fillSidebarMenu,()) { - CoreSuspender suspend; color_ostream_proxy out(Core::getInstance().getConsole()); bool call_native=true; - onWorkshopFillSidebarMenu(out,this,&call_native); + { + CoreSuspender suspend; + onWorkshopFillSidebarMenu(out,this,&call_native); + } if(call_native) INTERPOSE_NEXT(fillSidebarMenu)(); - postWorkshopFillSidebarMenu(out,this); + { + CoreSuspender suspend; + postWorkshopFillSidebarMenu(out,this); + } } }; IMPLEMENT_VMETHOD_INTERPOSE(furnace_hook, fillSidebarMenu); @@ -350,9 +355,11 @@ struct product_hook : item_product { return; } df::reaction* this_reaction=product->react; - CoreSuspender suspend; bool call_native=true; - onReactionCompleting(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items,&call_native); + { + CoreSuspender suspend; + onReactionCompleting(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items,&call_native); + } if(!call_native) return; @@ -361,8 +368,11 @@ struct product_hook : item_product { INTERPOSE_NEXT(produce)(unit, out_products, out_items, in_reag, in_items, quantity, skill, quality, entity, site, improv_items); if ( out_items->size() == out_item_count ) return; - //if it produced something, call the scripts - onReactionComplete(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items); + { + //if it produced something, call the scripts + CoreSuspender suspend; + onReactionComplete(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items); + } } }; @@ -374,9 +384,11 @@ struct item_hooks :df::item_actual { DEFINE_VMETHOD_INTERPOSE(void, contaminateWound,(df::unit* unit, df::unit_wound* wound, int32_t a1, int16_t a2)) { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onItemContaminateWound(out,this,unit,wound,a1,a2); + { + CoreSuspender suspend; + color_ostream_proxy out(Core::getInstance().getConsole()); + onItemContaminateWound(out,this,unit,wound,a1,a2); + } INTERPOSE_NEXT(contaminateWound)(unit,wound,a1,a2); } @@ -387,16 +399,20 @@ struct proj_item_hook: df::proj_itemst{ typedef df::proj_itemst interpose_base; DEFINE_VMETHOD_INTERPOSE(bool,checkImpact,(bool mode)) { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjItemCheckImpact(out,this,mode); + { + CoreSuspender suspend; + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjItemCheckImpact(out,this,mode); + } return INTERPOSE_NEXT(checkImpact)(mode); //returns destroy item or not? } DEFINE_VMETHOD_INTERPOSE(bool,checkMovement,()) { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjItemCheckMovement(out,this); + { + CoreSuspender suspend; + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjItemCheckMovement(out,this); + } return INTERPOSE_NEXT(checkMovement)(); } }; @@ -407,16 +423,20 @@ struct proj_unit_hook: df::proj_unitst{ typedef df::proj_unitst interpose_base; DEFINE_VMETHOD_INTERPOSE(bool,checkImpact,(bool mode)) { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjUnitCheckImpact(out,this,mode); + { + CoreSuspender suspend; + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjUnitCheckImpact(out,this,mode); + } return INTERPOSE_NEXT(checkImpact)(mode); //returns destroy item or not? } DEFINE_VMETHOD_INTERPOSE(bool,checkMovement,()) { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjUnitCheckMovement(out,this); + { + CoreSuspender suspend; + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjUnitCheckMovement(out,this); + } return INTERPOSE_NEXT(checkMovement)(); } }; From a1d343ad3e86e71fbd944822a93967c9551335de Mon Sep 17 00:00:00 2001 From: Myk Taylor Date: Tue, 31 Dec 2024 19:08:36 -0800 Subject: [PATCH 2/2] remove CoreSuspender from interpose methods --- library/include/VTableInterpose.h | 3 -- plugins/building-hacks.cpp | 1 - plugins/devel/eventExample.cpp | 1 - plugins/eventful.cpp | 67 ++++++++----------------------- plugins/overlay.cpp | 2 - plugins/rendermax/rendermax.cpp | 3 -- plugins/siege-engine.cpp | 1 - plugins/stockflow.cpp | 2 - 8 files changed, 17 insertions(+), 63 deletions(-) diff --git a/library/include/VTableInterpose.h b/library/include/VTableInterpose.h index 82c8675cf1..7886762396 100644 --- a/library/include/VTableInterpose.h +++ b/library/include/VTableInterpose.h @@ -45,9 +45,6 @@ namespace DFHack // You may define additional methods here, but NOT non-static fields DEFINE_VMETHOD_INTERPOSE(int, foo, (int arg)) { - // If needed by the code, claim the suspend lock. - // CoreSuspender suspend; - ... ... this->field ... // access fields of the df::someclass object ... int orig_retval = INTERPOSE_NEXT(foo)(arg); // call the original method diff --git a/plugins/building-hacks.cpp b/plugins/building-hacks.cpp index de0fdabda0..72c6e50f50 100644 --- a/plugins/building-hacks.cpp +++ b/plugins/building-hacks.cpp @@ -251,7 +251,6 @@ struct work_hook : df::building_workshopst{ { if(world->frame_counter % def->skip_updates == 0) { - CoreSuspender suspend; color_ostream_proxy out(Core::getInstance().getConsole()); onUpdateAction(out,this); } diff --git a/plugins/devel/eventExample.cpp b/plugins/devel/eventExample.cpp index 1fac7ecd6d..e0a1252c73 100644 --- a/plugins/devel/eventExample.cpp +++ b/plugins/devel/eventExample.cpp @@ -52,7 +52,6 @@ struct my_contaminate : df::item_actual { typedef df::item_actual interpose_base; DEFINE_VMETHOD_INTERPOSE(void, contaminateWound, (df::unit* unit, df::unit_wound* wound, uint8_t unk1, int16_t unk2)) { INTERPOSE_NEXT(contaminateWound)(unit,wound,unk1,unk2); - CoreSuspender suspend; Core::getInstance().print("contaminateWound: item=%d, unit=%d, wound attacker = %d, unk1 = %d, unk2 = %d\n", this->id, unit->id, wound->unit_id, (int32_t)unk1, (int32_t)unk2); } }; diff --git a/plugins/eventful.cpp b/plugins/eventful.cpp index 267119b92c..924b6720e9 100644 --- a/plugins/eventful.cpp +++ b/plugins/eventful.cpp @@ -301,16 +301,10 @@ struct workshop_hook : df::building_workshopst{ { color_ostream_proxy out(Core::getInstance().getConsole()); bool call_native=true; - { - CoreSuspender suspend; - onWorkshopFillSidebarMenu(out,this,&call_native); - } + onWorkshopFillSidebarMenu(out,this,&call_native); if(call_native) INTERPOSE_NEXT(fillSidebarMenu)(); - { - CoreSuspender suspend; - postWorkshopFillSidebarMenu(out,this); - } + postWorkshopFillSidebarMenu(out,this); } }; IMPLEMENT_VMETHOD_INTERPOSE(workshop_hook, fillSidebarMenu); @@ -321,16 +315,10 @@ struct furnace_hook : df::building_furnacest{ { color_ostream_proxy out(Core::getInstance().getConsole()); bool call_native=true; - { - CoreSuspender suspend; - onWorkshopFillSidebarMenu(out,this,&call_native); - } + onWorkshopFillSidebarMenu(out,this,&call_native); if(call_native) INTERPOSE_NEXT(fillSidebarMenu)(); - { - CoreSuspender suspend; - postWorkshopFillSidebarMenu(out,this); - } + postWorkshopFillSidebarMenu(out,this); } }; IMPLEMENT_VMETHOD_INTERPOSE(furnace_hook, fillSidebarMenu); @@ -356,10 +344,7 @@ struct product_hook : item_product { } df::reaction* this_reaction=product->react; bool call_native=true; - { - CoreSuspender suspend; - onReactionCompleting(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items,&call_native); - } + onReactionCompleting(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items,&call_native); if(!call_native) return; @@ -368,11 +353,8 @@ struct product_hook : item_product { INTERPOSE_NEXT(produce)(unit, out_products, out_items, in_reag, in_items, quantity, skill, quality, entity, site, improv_items); if ( out_items->size() == out_item_count ) return; - { - //if it produced something, call the scripts - CoreSuspender suspend; - onReactionComplete(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items); - } + //if it produced something, call the scripts + onReactionComplete(out,this_reaction,(df::reaction_product_itemst*)this,unit,in_items,in_reag,out_items); } }; @@ -384,11 +366,8 @@ struct item_hooks :df::item_actual { DEFINE_VMETHOD_INTERPOSE(void, contaminateWound,(df::unit* unit, df::unit_wound* wound, int32_t a1, int16_t a2)) { - { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onItemContaminateWound(out,this,unit,wound,a1,a2); - } + color_ostream_proxy out(Core::getInstance().getConsole()); + onItemContaminateWound(out,this,unit,wound,a1,a2); INTERPOSE_NEXT(contaminateWound)(unit,wound,a1,a2); } @@ -399,20 +378,14 @@ struct proj_item_hook: df::proj_itemst{ typedef df::proj_itemst interpose_base; DEFINE_VMETHOD_INTERPOSE(bool,checkImpact,(bool mode)) { - { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjItemCheckImpact(out,this,mode); - } + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjItemCheckImpact(out,this,mode); return INTERPOSE_NEXT(checkImpact)(mode); //returns destroy item or not? } DEFINE_VMETHOD_INTERPOSE(bool,checkMovement,()) { - { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjItemCheckMovement(out,this); - } + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjItemCheckMovement(out,this); return INTERPOSE_NEXT(checkMovement)(); } }; @@ -423,20 +396,14 @@ struct proj_unit_hook: df::proj_unitst{ typedef df::proj_unitst interpose_base; DEFINE_VMETHOD_INTERPOSE(bool,checkImpact,(bool mode)) { - { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjUnitCheckImpact(out,this,mode); - } + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjUnitCheckImpact(out,this,mode); return INTERPOSE_NEXT(checkImpact)(mode); //returns destroy item or not? } DEFINE_VMETHOD_INTERPOSE(bool,checkMovement,()) { - { - CoreSuspender suspend; - color_ostream_proxy out(Core::getInstance().getConsole()); - onProjUnitCheckMovement(out,this); - } + color_ostream_proxy out(Core::getInstance().getConsole()); + onProjUnitCheckMovement(out,this); return INTERPOSE_NEXT(checkMovement)(); } }; diff --git a/plugins/overlay.cpp b/plugins/overlay.cpp index 2920f80fb1..e40ec5cb29 100644 --- a/plugins/overlay.cpp +++ b/plugins/overlay.cpp @@ -51,8 +51,6 @@ static void overlay_interpose_lua(const char *fn_name, int nargs = 0, int nres = Lua::LuaLambda && res_lambda = Lua::DEFAULT_LUA_LAMBDA) { DEBUG(event).print("calling overlay lua function: '%s'\n", fn_name); - CoreSuspender guard; - color_ostream & out = Core::getInstance().getConsole(); auto L = Lua::Core::State; diff --git a/plugins/rendermax/rendermax.cpp b/plugins/rendermax/rendermax.cpp index 9832dc86bc..5515bb39a2 100644 --- a/plugins/rendermax/rendermax.cpp +++ b/plugins/rendermax/rendermax.cpp @@ -58,7 +58,6 @@ struct dwarmode_render_hook : viewscreen_dwarfmodest{ typedef df::viewscreen_dwarfmodest interpose_base; DEFINE_VMETHOD_INTERPOSE(void,render,()) { - CoreSuspender suspend; engine->preRender(); INTERPOSE_NEXT(render)(); engine->calculate(); @@ -71,7 +70,6 @@ struct dungeon_render_hook : viewscreen_dungeonmodest{ typedef df::viewscreen_dungeonmodest interpose_base; DEFINE_VMETHOD_INTERPOSE(void,render,()) { - CoreSuspender suspend; engine->preRender(); INTERPOSE_NEXT(render)(); engine->calculate(); @@ -296,7 +294,6 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan { case SC_VIEWSCREEN_CHANGED: { - CoreSuspender suspender; if(current_mode==MODE_LIGHT) { engine->clear(); diff --git a/plugins/siege-engine.cpp b/plugins/siege-engine.cpp index d38ab5a252..ca30fc76b4 100644 --- a/plugins/siege-engine.cpp +++ b/plugins/siege-engine.cpp @@ -1672,7 +1672,6 @@ struct projectile_hook : df::proj_itemst { return; auto L = Lua::Core::State; - CoreSuspender suspend; color_ostream_proxy out(Core::getInstance().getConsole()); df::unit *op_unit = getOperatorUnit(engine->bld, true); diff --git a/plugins/stockflow.cpp b/plugins/stockflow.cpp index 5445d026b7..1671d08991 100644 --- a/plugins/stockflow.cpp +++ b/plugins/stockflow.cpp @@ -112,8 +112,6 @@ class LuaHelper { bool stockpile_method(const char *method, building_stockpilest *sp) { // Combines the select_order and toggle_trigger method calls, // because they share the same signature. - CoreSuspender suspend; - auto L = Lua::Core::State; color_ostream_proxy out(Core::getInstance().getConsole());