securing thread-safe StaticConfig creations

supermerill/SuperSlicer#1110

src/libslic3r/Config.cpp

@@ -590,11 +590,11 @@ bool ConfigBase::set_deserialize_raw(const t_config_option_key &opt_key_src, con
         throw new UnknownOptionException(opt_key);
 
     bool ok = true;
-    if (!optdef->can_phony || value != "")
+    if (!optdef->can_phony || !value.empty())
         ok = opt->deserialize(value, append);
     //set phony status
     if (optdef->can_phony)
-        if(value == "")
+        if(value.empty())
             opt->phony = true;
         else
             opt->phony = false;

src/libslic3r/Config.hpp

@@ -1444,7 +1444,7 @@ public:
 
     std::string serialize() const override
     {
-        // as names is staic-initialized, it's thread safe
+        // as names are static-initialized, it's thread safe
         static t_config_enum_names names = ConfigOptionEnum<T>::create_enum_names();
         assert(static_cast<int>(this->value) < int(names.size()));
         return names[static_cast<int>(this->value)];

src/libslic3r/Preset.hpp

@@ -698,7 +698,7 @@ public:
     // Returns the config of the selected printer, or nullptr if no printer is selected.
     DynamicPrintConfig*     get_selected_printer_config() { return (m_idx_selected == size_t(-1)) ? nullptr : &(this->get_selected_printer().config); }
     // Returns the config of the selected printer, or nullptr if no printer is selected.
-    PrinterTechnology       get_selected_printer_technology() { return (m_idx_selected == size_t(-1)) ? PrinterTechnology::ptAny : this->get_selected_printer().printer_technology(); }
+    //PrinterTechnology       get_selected_printer_technology() { return (m_idx_selected == size_t(-1)) ? PrinterTechnology::ptAny : this->get_selected_printer().printer_technology(); }
 
     // Each physical printer can have a several related preset,
     // so, use the next functions to get an exact names of selections in the list:

src/libslic3r/PrintConfig.cpp

@@ -6182,20 +6182,6 @@ std::string FullPrintConfig::validate()
     return "";
 }
 
-// Declare the static caches for each StaticPrintConfig derived class.
-StaticPrintConfig::StaticCache<class Slic3r::PrintObjectConfig> PrintObjectConfig::s_cache_PrintObjectConfig;
-StaticPrintConfig::StaticCache<class Slic3r::PrintRegionConfig> PrintRegionConfig::s_cache_PrintRegionConfig;
-StaticPrintConfig::StaticCache<class Slic3r::MachineEnvelopeConfig> MachineEnvelopeConfig::s_cache_MachineEnvelopeConfig;
-StaticPrintConfig::StaticCache<class Slic3r::GCodeConfig>       GCodeConfig::s_cache_GCodeConfig;
-StaticPrintConfig::StaticCache<class Slic3r::PrintConfig>       PrintConfig::s_cache_PrintConfig;
-StaticPrintConfig::StaticCache<class Slic3r::FullPrintConfig>   FullPrintConfig::s_cache_FullPrintConfig;
-
-StaticPrintConfig::StaticCache<class Slic3r::SLAMaterialConfig>     SLAMaterialConfig::s_cache_SLAMaterialConfig;
-StaticPrintConfig::StaticCache<class Slic3r::SLAPrintConfig>        SLAPrintConfig::s_cache_SLAPrintConfig;
-StaticPrintConfig::StaticCache<class Slic3r::SLAPrintObjectConfig>  SLAPrintObjectConfig::s_cache_SLAPrintObjectConfig;
-StaticPrintConfig::StaticCache<class Slic3r::SLAPrinterConfig>      SLAPrinterConfig::s_cache_SLAPrinterConfig;
-StaticPrintConfig::StaticCache<class Slic3r::SLAFullPrintConfig>    SLAFullPrintConfig::s_cache_SLAFullPrintConfig;
-
 CLIActionsConfigDef::CLIActionsConfigDef()
 {
     ConfigOptionDef* def;

src/libslic3r/PrintConfig.hpp

@@ -13,11 +13,12 @@
 //            class PrintRegionConfig : public StaticPrintConfig
 //            class MachineEnvelopeConfig : public StaticPrintConfig
 //            class GCodeConfig : public StaticPrintConfig
-//                class PrintConfig : public MachineEnvelopeConfig, public GCodeConfig
+//                  class  : public MachineEnvelopeConfig, public GCodeConfig
 //                          class FullPrintConfig : PrintObjectConfig,PrintRegionConfig,PrintConfig
 //            class SLAPrintObjectConfig : public StaticPrintConfig
 //            class SLAMaterialConfig : public StaticPrintConfig
 //            class SLAPrinterConfig : public StaticPrintConfig
+//                  class SLAFullPrintConfig : public SLAPrinterConfig, public SLAPrintConfig, public SLAPrintObjectConfig, public SLAMaterialConfig
 //    class DynamicConfig : public virtual ConfigBase
 //        class DynamicPrintConfig : public DynamicConfig
 //            class DynamicPrintAndCLIConfig : public DynamicPrintConfig
@@ -160,6 +161,8 @@ template<> inline const t_config_enum_values& ConfigOptionEnum<PrinterTechnology
         {"FFF", ptFFF},
         {"SLA", ptSLA},
         {"SLS", ptSLS},
+        {"CNC", ptMill},
+        {"LSR", ptLaser},
     };
     return keys_map;
 }
@@ -493,8 +496,11 @@ protected:
     class StaticCache : public StaticCacheBase
     {
     public:
-        // Calling the constructor of m_defaults with 0 forces m_defaults to not run the initialization.
+        // To be called during the StaticCache setup.
-        StaticCache() : m_defaults(nullptr) {}
+        StaticCache(T* _defaults, std::function<void(T*, StaticCache<T>*)> initialize) : m_defaults(_defaults) {
+            initialize(_defaults , this);
+            this->finalize(_defaults);
+        }
         ~StaticCache() { delete m_defaults; m_defaults = nullptr; }
 
         bool                initialized() const { return ! m_keys.empty(); }
@@ -514,30 +520,32 @@ protected:
         const std::vector<std::string>& keys()      const { return m_keys; }
         const T&                        defaults()  const { return *m_defaults; }
 
+    private:
         // To be called during the StaticCache setup.
         // Collect option keys from m_map_name_to_offset,
         // assign default values to m_defaults.
-        void                finalize(T *defaults, const ConfigDef *defs)
+        void                finalize(T* defaults)
         {
+            assert(defaults != nullptr);
+            const ConfigDef* defs = m_defaults->def();
             assert(defs != nullptr);
             m_defaults = defaults;
             m_keys.clear();
             m_keys.reserve(m_map_name_to_offset.size());
-            for (const auto &kvp : defs->options) {
+            for (const auto& kvp : defs->options) {
                 // Find the option given the option name kvp.first by an offset from (char*)m_defaults.
-                ConfigOption *opt = this->optptr(kvp.first, m_defaults);
+                ConfigOption* opt = this->optptr(kvp.first, m_defaults);
                 if (opt == nullptr)
                     // This option is not defined by the ConfigBase of type T.
                     continue;
                 m_keys.emplace_back(kvp.first);
-                const ConfigOptionDef *def = defs->get(kvp.first);
+                const ConfigOptionDef* def = defs->get(kvp.first);
                 assert(def != nullptr);
                 if (def->default_value)
                     opt->set(def->default_value.get());
             }
         }
 
-    private:
         T                                  *m_defaults;
         std::vector<std::string>            m_keys;
     };
@@ -547,7 +555,7 @@ protected:
 public: \
     /* Overrides ConfigBase::optptr(). Find ando/or create a ConfigOption instance for a given name. */ \
     const ConfigOption*      optptr(const t_config_option_key &opt_key) const override \
-        {   const ConfigOption* opt = s_cache_##CLASS_NAME.optptr(opt_key, this); \
+        {   const ConfigOption* opt = config_cache().optptr(opt_key, this); \
             if (opt == nullptr && parent != nullptr) \
                 /*if not find, try with the parent config.*/ \
                 opt = parent->option(opt_key); \
@@ -555,28 +563,24 @@ public: \
         } \
     /* Overrides ConfigBase::optptr(). Find ando/or create a ConfigOption instance for a given name. */ \
     ConfigOption*            optptr(const t_config_option_key &opt_key, bool create = false) override \
-        { return s_cache_##CLASS_NAME.optptr(opt_key, this); } \
+        { return config_cache().optptr(opt_key, this); } \
     /* Overrides ConfigBase::keys(). Collect names of all configuration values maintained by this configuration store. */ \
-    t_config_option_keys     keys() const override { return s_cache_##CLASS_NAME.keys(); } \
+    t_config_option_keys     keys() const override { return config_cache().keys(); } \
-    const t_config_option_keys& keys_ref() const override { return s_cache_##CLASS_NAME.keys(); } \
+    const t_config_option_keys& keys_ref() const override { return config_cache().keys(); } \
-    static const CLASS_NAME& defaults() { initialize_cache(); return s_cache_##CLASS_NAME.defaults(); } \
+    static const CLASS_NAME& defaults() { return config_cache().defaults(); } \
 private: \
-    static void initialize_cache() \
+    static const StaticPrintConfig::StaticCache<CLASS_NAME>& config_cache() \
     { \
-        if (! s_cache_##CLASS_NAME.initialized()) { \
+        static StaticPrintConfig::StaticCache<CLASS_NAME> threadsafe_cache_##CLASS_NAME(new CLASS_NAME(1), \
-            CLASS_NAME *inst = new CLASS_NAME(1); \
+            [](CLASS_NAME *def, StaticPrintConfig::StaticCache<CLASS_NAME> *cache){ def->initialize(*cache, (const char*)def); } ); \
-            inst->initialize(s_cache_##CLASS_NAME, (const char*)inst); \
+        return threadsafe_cache_##CLASS_NAME; \
-            s_cache_##CLASS_NAME.finalize(inst, inst->def()); \
     } \
-    } \
-    /* Cache object holding a key/option map, a list of option keys and a copy of this static config initialized with the defaults. */ \
-    static StaticPrintConfig::StaticCache<CLASS_NAME> s_cache_##CLASS_NAME;
 
 #define STATIC_PRINT_CONFIG_CACHE(CLASS_NAME) \
     STATIC_PRINT_CONFIG_CACHE_BASE(CLASS_NAME) \
 public: \
     /* Public default constructor will initialize the key/option cache and the default object copy if needed. */ \
-    CLASS_NAME() { initialize_cache(); *this = s_cache_##CLASS_NAME.defaults(); } \
+    CLASS_NAME() { *this = config_cache().defaults(); } \
 protected: \
     /* Protected constructor to be called when compounded. */ \
     CLASS_NAME(int) {}
@@ -1227,15 +1231,15 @@ protected:
 class PrintConfig : public MachineEnvelopeConfig, public GCodeConfig
 {
     STATIC_PRINT_CONFIG_CACHE_DERIVED(PrintConfig)
-    PrintConfig() : MachineEnvelopeConfig(0), GCodeConfig(0) { initialize_cache(); *this = s_cache_PrintConfig.defaults(); }
+    PrintConfig() : MachineEnvelopeConfig(0), GCodeConfig(0) { *this = config_cache().defaults(); }
 public:
     double                          min_object_distance() const;
     static double                   min_object_distance(const ConfigBase *config, double height = 0);
 
     ConfigOptionBool                allow_empty_layers;
     ConfigOptionBool                avoid_crossing_perimeters;
-
+    ConfigOptionBool                avoid_crossing_not_first_layer;    
-    ConfigOptionBool                avoid_crossing_not_first_layer;    ConfigOptionFloatOrPercent      avoid_crossing_perimeters_max_detour;
+    ConfigOptionFloatOrPercent      avoid_crossing_perimeters_max_detour;
     ConfigOptionPoints              bed_shape;
     ConfigOptionInts                bed_temperature;
     ConfigOptionFloatOrPercent      bridge_acceleration;
@@ -1426,7 +1430,7 @@ class FullPrintConfig :
     public PrintConfig
 {
     STATIC_PRINT_CONFIG_CACHE_DERIVED(FullPrintConfig)
-    FullPrintConfig() : PrintObjectConfig(0), PrintRegionConfig(0), PrintConfig(0) { initialize_cache(); *this = s_cache_FullPrintConfig.defaults(); }
+    FullPrintConfig() : PrintObjectConfig(0), PrintRegionConfig(0), PrintConfig(0) { *this = config_cache().defaults(); }
 
 public:
     // Validate the FullPrintConfig. Returns an empty string on success, otherwise an error message is returned.
@@ -1739,7 +1743,7 @@ protected:
 class SLAFullPrintConfig : public SLAPrinterConfig, public SLAPrintConfig, public SLAPrintObjectConfig, public SLAMaterialConfig
 {
     STATIC_PRINT_CONFIG_CACHE_DERIVED(SLAFullPrintConfig)
-    SLAFullPrintConfig() : SLAPrinterConfig(0), SLAPrintConfig(0), SLAPrintObjectConfig(0), SLAMaterialConfig(0) { initialize_cache(); *this = s_cache_SLAFullPrintConfig.defaults(); }
+    SLAFullPrintConfig() : SLAPrinterConfig(0), SLAPrintConfig(0), SLAPrintObjectConfig(0), SLAMaterialConfig(0) { *this = config_cache().defaults(); }
 
 public:
     // Validate the SLAFullPrintConfig. Returns an empty string on success, otherwise an error message is returned.

src/slic3r/GUI/BackgroundSlicingProcess.cpp

@@ -497,11 +497,11 @@ void BackgroundSlicingProcess::reset_export()
 {
 	bool running = true;
 	{
-		// I don't know if it's safe to let m_mutex be lock whiole doing invalidate_step.
+		// I don't know if it's safe to let m_mutex be lock while doing invalidate_step.
 		// if so, please remove the braces.
 		std::unique_lock<std::mutex> lck(m_mutex);
 		running = this->running();
-		assert(!running);
+		//assert(!running);
 	}
 	if (!running) {
 		m_export_path.clear();

src/slic3r/GUI/Tab.cpp

@@ -3030,10 +3030,13 @@ void Tab::load_current_preset()
                     }
                     else {
                         int page_id = wxGetApp().tab_panel()->FindPage(tab);
+                        //TODO shouldn't happen, emit an error here.
+                        if (page_id >= 0 && page_id < wxGetApp().tab_panel()->GetPageCount()) {
                             wxGetApp().tab_panel()->GetPage(page_id)->Show(false);
                             wxGetApp().tab_panel()->RemovePage(page_id);
                         }
                     }
+                }
                 static_cast<TabPrinter*>(this)->m_printer_technology = printer_technology;
                 m_active_page = tmp_page;
             }
@@ -3056,7 +3059,7 @@ void Tab::load_current_preset()
             //update width/spacing links
             if (m_type == Preset::TYPE_PRINT) {
                 //verify that spacings are set
-                if (m_config->update_phony({ wxGetApp().plater()->config() })) {
+                if (m_config && m_config->update_phony({ wxGetApp().plater()->config() })) {
                     update_dirty();
                     reload_config();
                 }