Fix global multiprocessing.Lock

.github/workflows/crunch-lints.yml

@@ -26,5 +26,5 @@ jobs:
           pip install --upgrade flake8
       - name: Lints
         run: |
-          flake8 --ignore=E501,W503 src/crunch.py
+          flake8 src/crunch.py
           shellcheck --exclude=2046 src/*.sh

Makefile

@@ -76,7 +76,7 @@ test-shell:
 	shellcheck --exclude=2046 src/*.sh
 
 test-valid-png-output:
-	crunch testfiles/*.png
+	src/crunch.py testfiles/*.png
 	pngcheck testfiles/*-crunch.png
 	rm testfiles/*-crunch.png
 

src/crunch.py

@@ -20,10 +20,6 @@
 
 from multiprocessing import Lock, Pool, cpu_count
 
-# Locks
-stdstream_lock = Lock()
-logging_lock = Lock()
-
 # Application Constants
 VERSION = "5.0.0"
 VERSION_STRING = "crunch v" + VERSION
@@ -191,7 +187,15 @@ def main(argv):
     # ////////////////////////////////////
     print("Crunching ...")
 
+    # create locks
+    ss_lock = Lock()
+    log_lock = Lock()
+
     if len(png_path_list) == 1:
+        # the global locks are not necessary for single file processing
+        # but must be instantiated because the logging functions are
+        # used for single and multi-process execution
+        lock_init(ss_lock, log_lock)
         # there is only one PNG file, skip spawning of processes and just optimize it
         optimize_png(png_path_list[0])
     else:
@@ -210,18 +214,22 @@ def main(argv):
             f"Spawning {processes} processes to optimize {len(png_path_list)} "
             f"image files..."
         )
-        p = Pool(processes)
+
+        # create multiprocessing pool with global locks
+        # based on approach described in https://stackoverflow.com/a/25558333/2848172
+        # to address shared memory leak described in
+        # https://github.com/chrissimpkins/Crunch/issues/100
+        p = Pool(processes, initializer=lock_init, initargs=(ss_lock, log_lock))
+
         try:
             p.map(optimize_png, png_path_list)
         except Exception as e:
-            stdstream_lock.acquire()
             sys.stderr.write(f"-----{os.linesep}")
             sys.stderr.write(
                 f"{ERROR_STRING} Error detected during execution of the request."
                 f"{os.linesep}"
             )
             sys.stderr.write(f"{e}{os.linesep}")
-            stdstream_lock.release()
             if is_gui(argv):
                 log_error(str(e))
             sys.exit(1)
@@ -443,6 +451,16 @@ def is_valid_png(filepath):
     return signature == expected_signature
 
 
+def lock_init(ss_lock, log_lock):
+    # Based on approach described in
+    # https://stackoverflow.com/a/25558333/2848172
+    global stdstream_lock
+    global logging_lock
+
+    stdstream_lock = ss_lock
+    logging_lock = log_lock
+
+
 def log_error(errmsg):
     current_time = time.strftime("%m-%d-%y %H:%M:%S")
     logging_lock.acquire()

src/test_crunch_execution.py

@@ -234,6 +234,7 @@ def test_crunch_function_fix_filepath_args_two_nonpng_files():
 # optimize_png function
 
 def test_crunch_function_optimize_png_unoptimized_file():
+    setup_logging_locks()
     startpath = os.path.join("testfiles", "robot.png")
     testpath = os.path.join("testfiles", "robot-crunch.png")
     # cleanup any existing files from previous tests
@@ -250,6 +251,7 @@ def test_crunch_function_optimize_png_unoptimized_file():
 
 
 def test_crunch_function_optimize_png_preoptimized_file():
+    setup_logging_locks()
     startpath = os.path.join("testfiles", "cat-cr.png") # test a file that has previously been optimized
     testpath = os.path.join("testfiles", "cat-cr-crunch.png")
     # cleanup any existing files from previous tests
@@ -266,6 +268,7 @@ def test_crunch_function_optimize_png_preoptimized_file():
 
 
 def test_crunch_function_optimize_png_bad_filetype(capsys):
+    setup_logging_locks()
     with pytest.raises(CalledProcessError):
         startpath = os.path.join("src", "crunch.py")
         src.crunch.optimize_png(startpath)
@@ -500,6 +503,7 @@ def test_crunch_function_main_multi_file_with_service_flag():
 
 def test_crunch_log_error():
     setup_logging_path()
+    setup_logging_locks()
 
     logpath = src.crunch.LOGFILE_PATH
     src.crunch.log_error("This is a test error message")
@@ -515,6 +519,7 @@ def test_crunch_log_error():
 
 def test_crunch_log_info():
     setup_logging_path()
+    setup_logging_locks()
 
     logpath = src.crunch.LOGFILE_PATH
     src.crunch.log_info("This is a test info message")
@@ -529,7 +534,8 @@ def test_crunch_log_info():
 
 @pytest.mark.skipif(sys.platform != "darwin", reason="requires macOS platform")
 def test_crunch_log_from_main_with_service():
-    teardown_logging_path()
+    setup_logging_path()
+    setup_logging_locks()
 
     with pytest.raises(SystemExit) as exit_info:
         startpath1 = os.path.join("testfiles", "robot.png")
@@ -574,6 +580,15 @@ def setup_logging_path():
     if not os.path.isfile(src.crunch.LOGFILE_PATH):
         open(src.crunch.LOGFILE_PATH, "w").close()
 
+def setup_logging_locks():
+    from src.crunch import lock_init
+    from multiprocessing import Lock
+
+    ss_lock = Lock()
+    log_lock = Lock()
+
+    lock_init(ss_lock, log_lock)
+
 
 def teardown_logging_path():
     if os.path.isfile(src.crunch.LOGFILE_PATH):