Merge pull request #175 from pycompression/release_1.5.2

Release 1.5.2

Author: rhpvorderman

.github/release_checklist.md

@@ -5,6 +5,7 @@ Release checklist
   - [ ] Set version to a stable number.
   - [ ] Change current development version in `CHANGELOG.rst` to stable version.
   - [ ] Change the version in `__init__.py`
+- [ ] Check if the address sanitizer does not find any problems using `tox -e asan`
 - [ ] Merge the release branch into `main`.
 - [ ] Created an annotated tag with the stable version number. Include changes 
 from CHANGELOG.rst.

.readthedocs.yml

@@ -3,7 +3,16 @@ formats: []  # Do not build epub and pdf
 
 python:
   install:
-    - method: pip
-      path: .
-conda:
-  environment: docs/conda-environment.yml
+    - requirements: "requirements-docs.txt"
+    - method: "pip"
+      path: "."
+
+sphinx:
+  configuration: docs/conf.py
+
+build:
+  os: "ubuntu-22.04"
+  tools:
+    python: "3"
+  apt_packages:
+    - libisal-dev

CHANGELOG.rst

@@ -7,6 +7,15 @@ Changelog
 .. This document is user facing. Please word the changes in such a way
 .. that users understand how the changes affect the new version.
 
+version 1.5.2
+-----------------
++ Fix a bug where a filehandle remained opened when ``igzip_threaded.open``
+  was used for writing with a wrong compression level.
++ Fix a memory leak that occurred when an error was thrown for a gzip header
+  with the wrong magic numbers.
++ Fix a memory leak that occurred when isal_zlib.decompressobj was given a
+  wrong wbits value.
+
 version 1.5.1
 -----------------
 + Fix a memory leak in the GzipReader.readall implementation.

docs/conda-environment.yml

@@ -135,7 +135,7 @@ def build_isa_l():
 
 setup(
     name="isal",
-    version="1.5.1",
+    version="1.5.2",
     description="Faster zlib and gzip compatible compression and "
                 "decompression by providing python bindings for the ISA-L "
                 "library.",

setup.py

@@ -27,4 +27,4 @@
     "__version__"
 ]
 
-__version__ = "1.5.1"
+__version__ = "1.5.2"

src/isal/__init__.py

@@ -12,7 +12,7 @@
 import queue
 import struct
 import threading
-from typing import BinaryIO, List, Optional, Tuple
+from typing import List, Optional, Tuple
 
 from . import igzip, isal_zlib
 
@@ -56,20 +56,13 @@ def open(filename, mode="rb", compresslevel=igzip._COMPRESS_LEVEL_TRADEOFF,
                 threads = multiprocessing.cpu_count()
             except:  # noqa: E722
                 threads = 1
-    open_mode = mode.replace("t", "b")
-    if isinstance(filename, (str, bytes)) or hasattr(filename, "__fspath__"):
-        binary_file = builtins.open(filename, open_mode)
-    elif hasattr(filename, "read") or hasattr(filename, "write"):
-        binary_file = filename
-    else:
-        raise TypeError("filename must be a str or bytes object, or a file")
     if "r" in mode:
         gzip_file = io.BufferedReader(
-            _ThreadedGzipReader(binary_file, block_size=block_size))
+            _ThreadedGzipReader(filename, block_size=block_size))
     else:
         gzip_file = io.BufferedWriter(
             _ThreadedGzipWriter(
-                fp=binary_file,
+                filename,
                 block_size=block_size,
                 level=compresslevel,
                 threads=threads
@@ -81,10 +74,20 @@ def open(filename, mode="rb", compresslevel=igzip._COMPRESS_LEVEL_TRADEOFF,
     return gzip_file
 
 
+def open_as_binary_stream(filename, open_mode):
+    if isinstance(filename, (str, bytes)) or hasattr(filename, "__fspath__"):
+        binary_file = builtins.open(filename, open_mode)
+    elif hasattr(filename, "read") or hasattr(filename, "write"):
+        binary_file = filename
+    else:
+        raise TypeError("filename must be a str or bytes object, or a file")
+    return binary_file
+
+
 class _ThreadedGzipReader(io.RawIOBase):
-    def __init__(self, fp, queue_size=2, block_size=1024 * 1024):
-        self.raw = fp
-        self.fileobj = igzip._IGzipReader(fp, buffersize=8 * block_size)
+    def __init__(self, filename, queue_size=2, block_size=1024 * 1024):
+        self.raw = open_as_binary_stream(filename, "rb")
+        self.fileobj = igzip._IGzipReader(self.raw, buffersize=8 * block_size)
         self.pos = 0
         self.read_file = False
         self.queue = queue.Queue(queue_size)
@@ -193,15 +196,14 @@ class _ThreadedGzipWriter(io.RawIOBase):
     compressing and output is handled in one thread.
     """
     def __init__(self,
-                 fp: BinaryIO,
+                 filename,
                  level: int = isal_zlib.ISAL_DEFAULT_COMPRESSION,
                  threads: int = 1,
                  queue_size: int = 1,
                  block_size: int = 1024 * 1024,
                  ):
         self.lock = threading.Lock()
         self.exception: Optional[Exception] = None
-        self.raw = fp
         self.level = level
         self.previous_block = b""
         # Deflating random data results in an output a little larger than the
@@ -236,6 +238,7 @@ def __init__(self,
         self.running = False
         self._size = 0
         self._closed = False
+        self.raw = open_as_binary_stream(filename, "wb")
         self._write_gzip_header()
         self.start()
 

src/isal/igzip_threaded.py

@@ -793,6 +793,7 @@ isal_zlib_decompressobj_impl(PyObject *module, int wbits, PyObject *zdict)
     err = wbits_to_flag_and_hist_bits_inflate(wbits, &hist_bits, &flag);
     if (err < 0) {
         PyErr_Format(PyExc_ValueError, "Invalid wbits value: %d", wbits);
+        Py_DECREF(self);
         return NULL;
     }
     else if (err == 0) {
@@ -1683,9 +1684,11 @@ GzipReader_read_into_buffer(GzipReader *self, uint8_t *out_buffer, size_t out_bu
 
                     if (!(magic1 == 0x1f && magic2 == 0x8b)) {
                         Py_BLOCK_THREADS;
+                        PyObject *magic_obj = PyBytes_FromStringAndSize((char *)current_pos, 2);
                         PyErr_Format(BadGzipFile,
                             "Not a gzipped file (%R)", 
-                            PyBytes_FromStringAndSize((char *)current_pos, 2));
+                            magic_obj);
+                        Py_DECREF(magic_obj);
                         return -1;
                     };
                     uint8_t method = current_pos[2];

src/isal/isal_zlibmodule.c

@@ -99,7 +99,7 @@ def test_threaded_write_oversized_block_no_error(threads):
 @pytest.mark.parametrize("threads", [1, 3])
 def test_threaded_write_error(threads):
     f = igzip_threaded._ThreadedGzipWriter(
-        fp=io.BytesIO(), level=3,
+        io.BytesIO(), level=3,
         threads=threads, block_size=8 * 1024)
     # Bypass the write method which should not allow blocks larger than
     # block_size.
@@ -139,10 +139,11 @@ def test_writer_not_readable():
 
 
 def test_writer_wrong_level():
-    with pytest.raises(ValueError) as error:
-        igzip_threaded._ThreadedGzipWriter(io.BytesIO(), level=42)
-    error.match("Invalid compression level")
-    error.match("42")
+    with tempfile.NamedTemporaryFile("wb") as tmp:
+        with pytest.raises(ValueError) as error:
+            igzip_threaded.open(tmp.name, mode="wb", compresslevel=42)
+        error.match("Invalid compression level")
+        error.match("42")
 
 
 def test_writer_too_low_threads():

tests/test_igzip_threaded.py

@@ -22,6 +22,15 @@ commands =
     coverage html -i
     coverage xml -i
 
+[testenv:asan]
+setenv=
+    PYTHONDEVMODE=1
+    PYTHONMALLOC=malloc
+    CFLAGS=-lasan -fsanitize=address -fno-omit-frame-pointer
+allowlist_externals=bash
+commands=
+    bash -c 'export LD_PRELOAD=$(gcc -print-file-name=libasan.so) && printenv LD_PRELOAD && python -c "from isal import isal_zlib" && pytest tests'
+
 [testenv:compliance]
 deps=pytest
 commands=