Merge remote-tracking branch 'suokko/kittens_data_race_fix' into develop

library/include/Console.h

@@ -26,6 +26,7 @@ distribution.
 #include "Pragma.h"
 #include "Export.h"
 #include "ColorText.h"
+#include <atomic>
 #include <deque>
 #include <fstream>
 #include <assert.h>
@@ -163,6 +164,6 @@ namespace  DFHack
     private:
         Private * d;
         tthread::recursive_mutex * wlock;
-        bool inited;
+        std::atomic<bool> inited;
     };
 }

library/include/modules/Screen.h

@@ -36,6 +36,7 @@ distribution.
 #include "DataDefs.h"
 #include "df/graphic.h"
 #include "df/viewscreen.h"
+#include "df/zoom_commands.h"
 
 #include "modules/GuiHooks.h"
 
@@ -183,6 +184,9 @@ namespace DFHack
             return rect2d(df::coord2d(0,0), getWindowSize()-df::coord2d(1,1));
         }
 
+        /// Wrapper to call enabler->zoom_display from plugins
+        DFHACK_EXPORT void zoom(df::zoom_commands cmd);
+
         /// Returns the state of [GRAPHICS:YES/NO]
         DFHACK_EXPORT bool inGraphicsMode();
 

library/modules/Screen.cpp

@@ -90,6 +90,10 @@ df::coord2d Screen::getWindowSize()
     return df::coord2d(gps->dimx, gps->dimy);
 }
 
+void Screen::zoom(df::zoom_commands cmd) {
+    enabler->zoom_display(cmd);
+}
+
 bool Screen::inGraphicsMode()
 {
     return init && init->display.flag.is_set(init_display_flags::USE_GRAPHICS);

plugins/devel/kittens.cpp

@@ -1,3 +1,4 @@
+#include <atomic>
 #include <vector>
 #include <string>
 
@@ -24,13 +25,12 @@ DFHACK_PLUGIN_IS_ENABLED(is_enabled);
 REQUIRE_GLOBAL(ui);
 REQUIRE_GLOBAL(world);
 
-//FIXME: possible race conditions with calling kittens from the IO thread and shutdown from Core.
+std::atomic<bool> shutdown_flag{false};
-volatile bool shutdown_flag = false;
+std::atomic<bool> final_flag{true};
-volatile bool final_flag = true;
+std::atomic<bool> timering{false};
-bool timering = false;
+std::atomic<bool> trackmenu_flg{false};
-bool trackmenu_flg = false;
+std::atomic<uint8_t> trackpos_flg{0};
-bool trackpos_flg = false;
+std::atomic<uint8_t> statetrack{0};
-bool statetrack = false;
 int32_t last_designation[3] = {-30000, -30000, -30000};
 int32_t last_mouse[2] = {-1, -1};
 df::ui_sidebar_mode last_menu = df::ui_sidebar_mode::Default;
@@ -93,12 +93,10 @@ DFhackCExport command_result plugin_onstatechange(color_ostream &out, state_chan
 
 DFhackCExport command_result plugin_onupdate ( color_ostream &out )
 {
-    if(timering == true)
+    if(timering)
     {
         uint64_t time2 = GetTimeMs64();
-        // harmless potential data race here...
         uint64_t delta = time2-timeLast;
-        // harmless potential data race here...
         timeLast = time2;
         out.print("Time delta = %d ms\n", int(delta));
     }
@@ -135,30 +133,30 @@ DFhackCExport command_result plugin_onupdate ( color_ostream &out )
 
 command_result trackmenu (color_ostream &out, vector <string> & parameters)
 {
-    if(trackmenu_flg)
+    bool is_running = trackmenu_flg.exchange(false);
+    if(is_running)
     {
-        trackmenu_flg = false;
         return CR_OK;
     }
     else
     {
-        trackmenu_flg = true;
         is_enabled = true;
         last_menu = ui->main.mode;
         out.print("Menu: %d\n",last_menu);
+        trackmenu_flg = true;
         return CR_OK;
     }
 }
 command_result trackpos (color_ostream &out, vector <string> & parameters)
 {
-    trackpos_flg = !trackpos_flg;
+    trackpos_flg.fetch_xor(1);
     is_enabled = true;
     return CR_OK;
 }
 
 command_result trackstate ( color_ostream& out, vector< string >& parameters )
 {
-    statetrack = !statetrack;
+    statetrack.fetch_xor(1);
     return CR_OK;
 }
 
@@ -180,20 +178,19 @@ command_result colormods (color_ostream &out, vector <string> & parameters)
 
 command_result ktimer (color_ostream &out, vector <string> & parameters)
 {
-    if(timering)
+    bool is_running = timering.exchange(false);
+    if(is_running)
     {
-        timering = false;
         return CR_OK;
     }
     uint64_t timestart = GetTimeMs64();
     {
         CoreSuspender suspend;
+        uint64_t timeend = GetTimeMs64();
+        timeLast = timeend;
+        timering = true;
+        out.print("Time to suspend = %d ms\n", int(timeend - timestart));
     }
-    uint64_t timeend = GetTimeMs64();
-    out.print("Time to suspend = %d ms\n", int(timeend - timestart));
-    // harmless potential data race here...
-    timeLast = timeend;
-    timering = true;
     is_enabled = true;
     return CR_OK;
 }
@@ -255,7 +252,7 @@ command_result kittens (color_ostream &out, vector <string> & parameters)
     Console::color_value color = COLOR_BLUE;
     while(1)
     {
-        if(shutdown_flag)
+        if(shutdown_flag || !con.isInited())
         {
             final_flag = true;
             con.reset_color();

plugins/devel/zoom.cpp

@@ -7,6 +7,7 @@
 #include <map>
 #include <vector>
 #include "modules/Gui.h"
+#include "modules/Screen.h"
 #include "modules/World.h"
 #include "df/enabler.h"
 
@@ -52,7 +53,7 @@ command_result df_zoom (color_ostream &out, std::vector <std::string> & paramete
         return CR_FAILURE;
     }
     df::zoom_commands cmd = zcmap[parameters[0]];
-    enabler->zoom_display(cmd);
+    Screen::zoom(cmd);
     if (cmd == df::zoom_commands::zoom_fullscreen)
         enabler->fullscreen = !enabler->fullscreen;
     return CR_OK;