diff --git a/.github/release_checklist.md b/.github/release_checklist.md index c47123ba..7093fe46 100644 --- a/.github/release_checklist.md +++ b/.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. diff --git a/.readthedocs.yml b/.readthedocs.yml index 2866edaf..910b7650 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -3,7 +3,16 @@ formats: [] # Do not build epub and pdf python: install: - - method: pip - path: . -conda: - environment: docs/conda-environment.yml \ No newline at end of file + - requirements: "requirements-docs.txt" + - method: "pip" + path: "." + +sphinx: + configuration: docs/conf.py + +build: + os: "ubuntu-22.04" + tools: + python: "3" + apt_packages: + - libisal-dev diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0f1eaff3..3e936f62 100644 --- a/CHANGELOG.rst +++ b/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. diff --git a/docs/conda-environment.yml b/docs/conda-environment.yml deleted file mode 100644 index a23cb482..00000000 --- a/docs/conda-environment.yml +++ /dev/null @@ -1,12 +0,0 @@ -name: rtd -channels: - - conda-forge - - defaults -dependencies: - - isa-l - - python >=3.6 - - sphinx - - setuptools - - pip: - - sphinx-rtd-theme - - sphinx-argparse \ No newline at end of file diff --git a/setup.py b/setup.py index 9bc284fd..22c16742 100644 --- a/setup.py +++ b/setup.py @@ -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.", diff --git a/src/isal/__init__.py b/src/isal/__init__.py index 9306adfc..8a39a81a 100644 --- a/src/isal/__init__.py +++ b/src/isal/__init__.py @@ -27,4 +27,4 @@ "__version__" ] -__version__ = "1.5.1" +__version__ = "1.5.2" diff --git a/src/isal/igzip_threaded.py b/src/isal/igzip_threaded.py index 20142e7a..dd34e704 100644 --- a/src/isal/igzip_threaded.py +++ b/src/isal/igzip_threaded.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,7 +196,7 @@ 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, @@ -201,7 +204,6 @@ def __init__(self, ): 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() diff --git a/src/isal/isal_zlibmodule.c b/src/isal/isal_zlibmodule.c index 257260f8..2c5c8a92 100644 --- a/src/isal/isal_zlibmodule.c +++ b/src/isal/isal_zlibmodule.c @@ -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]; diff --git a/tests/test_igzip_threaded.py b/tests/test_igzip_threaded.py index a0f581c6..be6c3ca4 100644 --- a/tests/test_igzip_threaded.py +++ b/tests/test_igzip_threaded.py @@ -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(): diff --git a/tox.ini b/tox.ini index c94c6690..256ee95e 100644 --- a/tox.ini +++ b/tox.ini @@ -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=