Copy the parameters in _make_estimator to prevent segfault caused by concurrent accesses to the same Criterion instance

aadamson · aadamson · commit bb91401d3a0f · 2020-12-09T16:09:58.000-05:00
diff --git a/sklearn/ensemble/_base.py b/sklearn/ensemble/_base.py
@@ -4,6 +4,7 @@
 # License: BSD 3 clause
 
 from abc import ABCMeta, abstractmethod
+import copy
 import numbers
 from typing import List
 
@@ -148,7 +149,7 @@ def _make_estimator(self, append=True, random_state=None):
         sub-estimators.
         """
         estimator = clone(self.base_estimator_)
-        estimator.set_params(**{p: getattr(self, p)
+        estimator.set_params(**{p: copy.deepcopy(getattr(self, p))
                                 for p in self.estimator_params})
 
         if random_state is not None:
diff --git a/sklearn/ensemble/tests/test_forest.py b/sklearn/ensemble/tests/test_forest.py
@@ -1377,3 +1377,23 @@ def test_little_tree_with_small_max_samples(ForestClass):
 
     msg = "Tree without `max_samples` restriction should have more nodes"
     assert tree1.node_count > tree2.node_count, msg
+
+
+@pytest.mark.parametrize(
+    'name', FOREST_REGRESSORS
+)
+def test_mse_criterion_object_segfault_smoke_test(name):
+    # This test exists to verify that we can successfully run all forest
+    # regressors with a criterion object without triggering a seg fault
+    # (#12623)
+    from sklearn.tree._classes import CRITERIA_REG
+
+    X = np.random.random((1000, 3))
+    y = np.random.random((1000, 1))
+
+    n_samples, n_outputs = y.shape
+    mse_criterion = CRITERIA_REG['mse'](n_outputs, n_samples)
+    est = FOREST_REGRESSORS[name](n_estimators=2, n_jobs=-1,
+                                  criterion=mse_criterion)
+
+    est.fit(X, y)
