PR comments

Author: rozyczko

EasyReflectometryApp/Backends/Py/analysis.py

@@ -123,12 +123,9 @@ def fittingStartStop(self) -> None:
 
     def _start_threaded_fit(self) -> None:
         """Start fitting in a background thread."""
-        # Reset flags and prepare for fit
+        # Reset flags and prepare for fit using proper encapsulation
         self._fitting_logic.reset_stop_flag()
-        self._fitting_logic._running = True
-        self._fitting_logic._finished = False
-        self._fitting_logic._show_results_dialog = False
-        self._fitting_logic._fit_error_message = None
+        self._fitting_logic.prepare_for_threaded_fit()
         self.fittingChanged.emit()
 
         # Prepare fit data for all experiments

EasyReflectometryApp/Backends/Py/logic/fitting.py

@@ -88,6 +88,17 @@ def reset_stop_flag(self) -> None:
         self._stop_requested = False
         self._fit_cancelled = False
 
+    def prepare_for_threaded_fit(self) -> None:
+        """Prepare state for a new threaded fit.
+
+        This method sets the internal state flags to indicate a fit is starting.
+        Call this before launching the background thread.
+        """
+        self._running = True
+        self._finished = False
+        self._show_results_dialog = False
+        self._fit_error_message = None
+
     def prepare_threaded_fit(self, minimizers_logic: 'Minimizers') -> tuple:
         """Prepare data for threaded fitting.
 
@@ -112,6 +123,20 @@ def prepare_threaded_fit(self, minimizers_logic: 'Minimizers') -> tuple:
             # Prepare data arrays for all experiments
             x_data = [experiments[idx].x for idx in experiments]
             y_data = [experiments[idx].y for idx in experiments]
+
+            # Validate error values before computing weights to avoid division by zero
+            import numpy as np
+
+            for idx in experiments:
+                ye = experiments[idx].ye
+                if np.any(ye == 0):
+                    exp_name = experiments[idx].name if hasattr(experiments[idx], 'name') else f'index {idx}'
+                    self._fit_error_message = f'Experiment {exp_name} has zero error values which would cause division by zero'
+                    self._running = False
+                    self._finished = True
+                    self._show_results_dialog = True
+                    return None, None, None, None, None
+
             weights = [1.0 / experiments[idx].ye for idx in experiments]
 
             # Method is optional in fit() - pass None to use minimizer's default

EasyReflectometryApp/Backends/Py/workers/fitter_worker.py

@@ -94,7 +94,10 @@ def run(self) -> None:
             method = getattr(self._fitter, self._method_name)
             result = method(*self._args, **self._kwargs)
 
-            # Check if stop was requested during execution
+            # NOTE: This check only catches stop requests that occurred AFTER the fit
+            # completed but before we emit the result. It does NOT interrupt the fitting
+            # algorithm mid-execution since lmfit/scipy don't support cancellation callbacks.
+            # The effective cancellation window is only before the fit starts (checked above).
             if self._stop_requested:
                 self.failed.emit('Fitting cancelled by user')
                 return
@@ -119,9 +122,26 @@ def stop(self) -> None:
         This sets a flag that is checked during execution and also
         terminates the thread if it's still running. Call wait() after
         this to ensure proper thread cleanup.
+
+        .. warning::
+            DANGEROUS: This method uses QThread.terminate() which is strongly
+            discouraged by Qt documentation. It can:
+            - Leave mutex locks held indefinitely causing deadlocks
+            - Corrupt data structures mid-operation
+            - Prevent proper cleanup of resources (especially numpy arrays, scipy internals)
+            - Cause memory leaks and undefined behavior
+
+            The fitting libraries (lmfit, scipy) do not support graceful cancellation.
+            The stop flag is only effective BEFORE the fit starts - once the fitting
+            algorithm is running, it cannot be interrupted cleanly.
+
+            See THREAD_TERMINATION_WARNING.md for details on known issues and
+            potential future improvements (e.g., using subprocess instead of QThread).
         """
         self._stop_requested = True
         if self.isRunning():
+            # WARNING: terminate() is dangerous but necessary since fitting
+            # libraries don't support graceful cancellation. See docstring above.
             self.terminate()
             self.wait()