Merged in memory (pull request #488)

Remove memory leaks

Approved-by: Randy Taylor
Approved-by: Hasan Ammar

bitbucket-pipelines.yml

@@ -38,11 +38,9 @@ definitions:
               - "*.rst"
     - step: &cbct-tests
         name: Run CBCT Tests
-        size: 2x
         script:
           # set up memory monitoring
-          - apt-get update
-          - apt-get install -y jq
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
           - chmod +x memory_monitor.sh
           - nohup ./memory_monitor.sh &
           - MONITOR_PID=$!
@@ -62,7 +60,16 @@ definitions:
     - step: &quart-tests
         name: Run Quart Tests
         script:
+          - # set up memory monitoring
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
+          - chmod +x memory_monitor.sh
+          - nohup ./memory_monitor.sh &
+          - MONITOR_PID=$!
           - uv run pytest tests_basic/test_quart.py --cov=pylinac.quart --cov-report term --junitxml=./test-reports/pytest_results.xml
+          # clean up memory monitor
+          - kill $MONITOR_PID
+        artifacts:
+          - memory_usage.log
         caches:
           - testfiles
         condition:
@@ -75,7 +82,16 @@ definitions:
     - step: &acr-tests
         name: Run ACR CT/MRI Tests
         script:
+          - # set up memory monitoring
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
+          - chmod +x memory_monitor.sh
+          - nohup ./memory_monitor.sh &
+          - MONITOR_PID=$!
           - uv run pytest tests_basic/test_acr.py --cov=pylinac.acr --cov-report term --junitxml=./test-reports/pytest_results.xml
+          # clean up memory monitor
+          - kill $MONITOR_PID
+        artifacts:
+          - memory_usage.log
         caches:
           - testfiles
         condition:
@@ -88,7 +104,16 @@ definitions:
     - step: &cheese-tests
         name: Run Cheese Phantom Tests
         script:
+          - # set up memory monitoring
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
+          - chmod +x memory_monitor.sh
+          - nohup ./memory_monitor.sh &
+          - MONITOR_PID=$!
           - uv run pytest tests_basic/test_cheese.py --cov=pylinac.cheese --cov-report term --junitxml=./test-reports/pytest_results.xml
+          # clean up memory monitor
+          - kill $MONITOR_PID
+        artifacts:
+          - memory_usage.log
         caches:
           - testfiles
         condition:
@@ -101,7 +126,16 @@ definitions:
     - step: &planar-tests
         name: Run Planar Imaging Tests
         script:
+          - # set up memory monitoring
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
+          - chmod +x memory_monitor.sh
+          - nohup ./memory_monitor.sh &
+          - MONITOR_PID=$!
           - uv run pytest tests_basic/test_planar_imaging.py --cov-report term --junitxml=./test-reports/pytest_results.xml
+          # clean up memory monitor
+          - kill $MONITOR_PID
+        artifacts:
+          - memory_usage.log
         caches:
           - testfiles
         condition:
@@ -149,7 +183,16 @@ definitions:
     - step: &picket-fence-tests
         name: Run Picket Fence Tests
         script:
+          - # set up memory monitoring
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
+          - chmod +x memory_monitor.sh
+          - nohup ./memory_monitor.sh &
+          - MONITOR_PID=$!
           - uv run pytest tests_basic/test_picketfence.py --cov-report term --junitxml=./test-reports/pytest_results.xml
+          # clean up memory monitor
+          - kill $MONITOR_PID
+        artifacts:
+          - memory_usage.log
         caches:
           - testfiles
         condition:
@@ -197,11 +240,9 @@ definitions:
               - "pylinac/vmat.py"
     - step: &winston-lutz-tests
         name: Run Winston-Lutz Tests
-        size: 2x
         script:
             # set up memory monitoring
-          - apt-get update
-          - apt-get install -y jq
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
           - chmod +x memory_monitor.sh
           - nohup ./memory_monitor.sh &
           - MONITOR_PID=$!
@@ -218,11 +259,9 @@ definitions:
           - memory_usage.log
     - step: &winston-lutz-mtmf-tests
         name: Run Winston-Lutz Multi-Target Multi-Field Tests
-        size: 2x
         script:
           # set up memory monitoring
-          - apt-get update
-          - apt-get install -y jq
+          - apt-get update && apt-get install -y --no-install-recommends procps jq
           - chmod +x memory_monitor.sh
           - nohup ./memory_monitor.sh &
           - MONITOR_PID=$!

docs/source/changelog.rst

@@ -10,6 +10,15 @@ Legend
 * :bdg-primary:`Refactor` denotes a code refactor; usually this means an efficiency boost or code cleanup.
 * :bdg-danger:`Change` denotes a change that may break existing code.
 
+v 3.30.0
+--------
+
+Image Metrics
+^^^^^^^^^^^^^
+
+* There was a memory bug when computing metrics. This shouldn't affect
+  one-off computations, but could affect long-running processes. This has been fixed.
+
 v 3.29.0
 --------
 

memory_monitor.sh

@@ -3,27 +3,47 @@
 # Configuration
 LOG_FILE="memory_usage.log"
 ACTIVE_TESTS_FILE="active_tests.json"
-# Dynamically set TEMP_DIR based on TMPDIR environment variable; default to /tmp if TMPDIR is not set
 TEMP_DIR="${TMPDIR:-/tmp}"
 POLL_INTERVAL=10  # Polling interval in seconds
 
-# Initialize the log file with headers
-echo "Timestamp,MemoryUsage(MB),MemoryLimit(MB),ActiveTests,TempDirSize(MB)" > "$LOG_FILE"
+# Initialize the log file with headers if it doesn't exist
+if [ ! -f "$LOG_FILE" ]; then
+    echo "Timestamp,MemoryUsage(MB),MemoryLimit(MB),ActiveTests,TempDirSize(MB)" > "$LOG_FILE"
+fi
 
 while true; do
     # Capture the current timestamp
     TIMESTAMP=$(date +"%Y-%m-%d %H:%M:%S")
 
-    # Read memory usage and limit
+    # Determine cgroup version
+    CGROUP_VERSION=0
     if [ -f /sys/fs/cgroup/memory/memory.usage_in_bytes ] && [ -f /sys/fs/cgroup/memory/memory.limit_in_bytes ]; then
+        CGROUP_VERSION=1
+    elif [ -f /sys/fs/cgroup/memory.current ] && [ -f /sys/fs/cgroup/memory.max ]; then
+        CGROUP_VERSION=2
+    fi
+
+    # Read memory usage and limit based on cgroup version
+    if [ "$CGROUP_VERSION" -eq 1 ]; then
+        # cgroup v1
         MEMORY_USAGE=$(cat /sys/fs/cgroup/memory/memory.usage_in_bytes 2>/dev/null || echo 0)
         MEMORY_LIMIT=$(cat /sys/fs/cgroup/memory/memory.limit_in_bytes 2>/dev/null || echo 0)
+    elif [ "$CGROUP_VERSION" -eq 2 ]; then
+        # cgroup v2
+        MEMORY_USAGE=$(cat /sys/fs/cgroup/memory.current 2>/dev/null || echo 0)
+        MEMORY_LIMIT=$(cat /sys/fs/cgroup/memory.max 2>/dev/null || echo 0)
+        # Handle cases where memory.max is "max"
+        if [ "$MEMORY_LIMIT" == "max" ]; then
+            # Fetch total memory from /proc/meminfo
+            MEMORY_LIMIT=$(grep MemTotal /proc/meminfo | awk '{print $2 * 1024}' || echo 0)
+        fi
     else
         # Fallback for systems without cgroup memory files
         MEMORY_USAGE=$(free -b | awk '/Mem:/ {print $3}' || echo 0)
         MEMORY_LIMIT=$(free -b | awk '/Mem:/ {print $2}' || echo 0)
     fi
 
+    # Convert bytes to megabytes
     MEMORY_USAGE_MB=$((MEMORY_USAGE / 1024 / 1024))
     MEMORY_LIMIT_MB=$((MEMORY_LIMIT / 1024 / 1024))
 
@@ -40,12 +60,7 @@ while true; do
     fi
 
     # Determine the temporary directory to monitor
-    # Prefer TMPDIR if set; else default to /tmp
-    if [ -n "$TMPDIR" ]; then
-        CURRENT_TEMP_DIR="$TMPDIR"
-    else
-        CURRENT_TEMP_DIR="/tmp"
-    fi
+    CURRENT_TEMP_DIR="$TEMP_DIR"
 
     # Calculate the size of the temporary directory in MB
     if [ -d "$CURRENT_TEMP_DIR" ]; then

pylinac/core/typing.py

@@ -1,3 +1,5 @@
+"""Typing utilities for Pylinac."""
+
 from __future__ import annotations
 
 from typing import Union

pylinac/metrics/image.py

@@ -2,6 +2,7 @@
 
 import math
 import typing
+import weakref
 from abc import ABC, abstractmethod
 from typing import Any, Callable
 
@@ -54,7 +55,7 @@ def inject_image(self, image: BaseImage):
             image, self.image_compatibility
         ):
             raise TypeError(f"Image must be one of {self.image_compatibility}")
-        self.image = image
+        self.image = weakref.proxy(image)
 
     def context_calculate(self) -> Any:
         """Calculate the metric. This also checks the image hash to attempt to ensure no changes were made."""

tests_basic/test_acr.py

@@ -91,6 +91,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.ct
 
     def test_plot_images(self):
         """Test that saving an image does something."""
@@ -151,6 +152,12 @@ def setUpClass(cls):
             scaling_factor=cls.scaling_factor,
         )
 
+    @classmethod
+    def tearDownClass(cls):
+        plt.close("all")
+        del cls.ct
+        super().tearDownClass()
+
     def test_roll(self):
         self.assertAlmostEqual(self.ct.catphan_roll, self.phantom_roll, delta=0.3)
 
@@ -369,6 +376,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.mri
 
     def test_plot_images(self):
         """Test that saving an image does something."""
@@ -431,6 +439,12 @@ def setUpClass(cls):
             scaling_factor=cls.scaling_factor,
         )
 
+    @classmethod
+    def tearDownClass(cls):
+        plt.close("all")
+        del cls.mri
+        super().tearDownClass()
+
     def test_roll(self):
         self.assertAlmostEqual(self.mri.catphan_roll, self.phantom_roll, delta=0.3)
 

tests_basic/test_cbct.py

@@ -307,6 +307,11 @@ class TestCustomHUValues(TestCase):
     def setUpClass(cls):
         cls.cbct = CatPhan504.from_demo_images()
 
+    @classmethod
+    def tearDownClass(cls):
+        plt.close("all")
+        del cls.cbct
+
     def test_override(self):
         self.cbct.analyze(expected_hu_values={"Air": -1000, "Poly": 321})
         self.assertEqual(self.cbct.ctp404.rois["Air"].nominal_val, -1000)
@@ -331,6 +336,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.cbct
 
     def test_save_image(self):
         """Test that saving an image does something."""
@@ -427,6 +433,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.cbct
         super().tearDownClass()
 
     def test_slice_thickness(self):

tests_basic/test_cheese.py

@@ -44,6 +44,11 @@ def setUpClass(cls) -> None:
         cls.cheese = TomoCheese.from_demo_images()
         cls.cheese.analyze()
 
+    @classmethod
+    def tearDownClass(cls) -> None:
+        plt.close("all")
+        del cls.cheese
+
     def test_results_as_str(self):
         assert isinstance(self.cheese.results(), str)
 
@@ -128,6 +133,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.cheese
 
     def test_save_pdf(self):
         # shouldn't raise
@@ -203,6 +209,8 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.cheese
+        super().tearDownClass()
 
     def test_slice_locations(self):
         """Test the locations of the slices of interest."""

tests_basic/test_field_profile_analysis.py

@@ -80,6 +80,12 @@ def setUpClass(cls):
             print(cls.fa.results())
         super().setUpClass()
 
+    @classmethod
+    def tearDownClass(cls) -> None:
+        plt.close("all")
+        del cls.fa
+        super().tearDownClass()
+
     def test_x_field_size(self):
         self.assertAlmostEqual(
             self.fa.results_data().x_metrics["Field Width (mm)"],

tests_basic/test_picketfence.py

@@ -134,6 +134,11 @@ def setUpClass(cls):
         cls.pf = PicketFence.from_demo_image()
         cls.pf.analyze()
 
+    @classmethod
+    def tearDownClass(cls):
+        plt.close("all")
+        del cls.pf
+
     def test_bad_tolerance_values(self):
         self.assertRaises(ValueError, self.pf.analyze, 0.2, 0.3)
 
@@ -419,6 +424,7 @@ def setUpClass(cls):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.pf
 
     def test_plotting(self):
         self.pf.plot_analyzed_image()
@@ -539,6 +545,12 @@ def setUpClass(cls):
                 nominal_gap_mm=cls.nominal_gap_mm,
             )
 
+    @classmethod
+    def tearDownClass(cls):
+        plt.close("all")
+        del cls.pf
+        super().tearDownClass()
+
     def test_passed(self):
         self.assertEqual(self.pf.passed, self.passes)
 
@@ -592,7 +604,7 @@ def setUpClass(cls):
 
     @classmethod
     def tearDownClass(cls):
-        pass  # override delete behavior
+        del cls.pf  # override delete behavior
 
     def test_demo_lower_tolerance(self):
         pf = PicketFence.from_demo_image()

tests_basic/test_planar_imaging.py

@@ -240,6 +240,7 @@ def preprocess(cls, instance):
     @classmethod
     def tearDownClass(cls):
         plt.close("all")
+        del cls.instance
         super().tearDownClass()
 
     def test_bad_inversion_recovers(self):