Skip to content

Two redundant reference leaks (no measurable memory growth) #1481

Description

@K-ANOY

Two places drop an owned reference without releasing it. Neither grows memory in practice — in both cases the leaked object is a singleton that stays alive for the life of the process regardless. They are correctness cleanups, not memory problems.


1. blob_filter_stream_write() / blob_filter_stream_close(): callback results

File: src/blob.c, functions blob_filter_stream_write (line 160) and blob_filter_stream_close (line 201)

In blob_filter_stream_write() the py_queue.put() result is released but the following py_ready.set() result is not:

result = PyObject_CallMethod(stream->py_queue, "put", "y#", pos, chunk_size);
if (result == NULL)
{
    ...
}
Py_DECREF(result);
result = PyObject_CallMethod(stream->py_ready, "set", NULL);
if (result == NULL)
{
    ...
}
pos += chunk_size;   /* result leaked, once per chunk */

blob_filter_stream_close() leaks both of its results; the first is overwritten without being released:

result = PyObject_CallMethod(stream->py_closed, "set", NULL);
if (result == NULL)
{
    ...
}
result = PyObject_CallMethod(stream->py_ready, "set", NULL);   /* first result leaked */
if (result == NULL)
{
    ...
}
/* second result leaked */

PyObject_CallMethod() returns a new reference in each case.

The leaked objects are whatever the caller's put() / set() methods return. The only caller is _BlobIO (pygit2/blob.py), which passes a real queue.Queue and two threading.Event objects, so the return value is always None — immortal on Python 3.12+ and never deallocated on older versions either.

Blob__write_to_queue parses its arguments as "OOO|nzIO" with no type check, so a caller passing objects whose put() / set() return something else would leak for real, but _write_to_queue is private and _BlobIO is its only user.

Suggested fix

Py_DECREF(result) after each successful call:

result = PyObject_CallMethod(stream->py_ready, "set", NULL);
if (result == NULL) {
    ...
}
Py_DECREF(result);

In blob_filter_stream_close() both calls are attempted regardless of the first one's outcome, so initialize result to NULL and Py_XDECREF it before each overwrite.


2. _cache_enums(): imported module

File: src/pygit2.c, function _cache_enums (line 377)

PyObject *enums = PyImport_ImportModule("pygit2.enums");
if (enums == NULL) {
    return NULL;
}

#define CACHE_PYGIT2_ENUM(name) do { \
    name##Enum = PyObject_GetAttrString(enums, #name); \
    if (name##Enum == NULL) { goto fail; } \
} while (0)

    CACHE_PYGIT2_ENUM(DeltaStatus);
    ...
    CACHE_PYGIT2_ENUM(ReferenceType);

#undef CACHE_PYGIT2_ENUM

    Py_RETURN_NONE;

fail:
    Py_DECREF(enums);
    forget_enums();
    return NULL;

PyImport_ImportModule() returns a new reference. The enum classes are stored in separate globals, so the module reference is not needed after caching. The fail: path releases it; the success path does not.

The module is in sys.modules either way, so nothing is ever actually freed, and _cache_enums() normally runs once at import. It is exposed as a module-level function, so repeated calls would inflate the refcount of that one module object without allocating anything.

Suggested fix

    Py_DECREF(enums);
    Py_RETURN_NONE;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions