From 933eeb0b4b66870c784f0a21aaf15044cd88ff74 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Mon, 9 Nov 2026 21:38:20 -0800 Subject: [PATCH] [cel-expr-python] Check for null returns when converting CEL values to Python objects. Previously, `PyCelFunctionAdapter::Invoke` and the list/map branches of `CelValueToPyObject` did not check whether nested `CelValueToPyObject` calls returned `nullptr`, leading to `SIGSEGV`s or leaked container references when an element could not be converted to a Python object. Check all allocation and conversion results and propagate errors cleanly. PiperOrigin-RevId: 1017903433 --- cel_expr_python/BUILD | 11 ++ cel_expr_python/cel_parallel_test.py | 23 +--- cel_expr_python/cel_test.py | 146 ++++++++++++++++++++------ cel_expr_python/leak_check.py | 137 ++++++++++++++++++++++++ cel_expr_python/py_cel_function.cc | 31 +++++- cel_expr_python/py_cel_value.cc | 112 +++++++++++++------- cel_expr_python/py_message_factory.cc | 6 +- release/build_wheel.sh | 2 +- release/cel_basic_test.py | 1 + release/kokoro/release_linux.sh | 2 +- release/kokoro/release_macos.sh | 2 +- release/kokoro/release_windows.bat | 3 + release/pyproject.toml | 2 +- 13 files changed, 375 insertions(+), 103 deletions(-) create mode 100644 cel_expr_python/leak_check.py diff --git a/cel_expr_python/BUILD b/cel_expr_python/BUILD index 02c8f9c..0a2cdd4 100644 --- a/cel_expr_python/BUILD +++ b/cel_expr_python/BUILD @@ -1,5 +1,6 @@ load("@pybind11_bazel//:build_defs.bzl", "pybind_extension", "pybind_library") load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_python//python:py_library.bzl", "py_library") load("@rules_python//python:py_test.bzl", "py_test") package(default_visibility = ["//visibility:public"]) @@ -150,6 +151,13 @@ cc_library( ], ) +py_library( + name = "leak_check", + testonly = True, + srcs = ["leak_check.py"], + visibility = [":__subpackages__"], +) + py_test( name = "cel_test", srcs = ["cel_test.py"], @@ -157,8 +165,10 @@ py_test( ":cel", ], deps = [ + ":leak_check", "//testing:proto2_test_all_types_py_pb2", "@com_google_absl_py//absl/testing:absltest", + "@com_google_absl_py//absl/testing:parameterized", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:protobuf_python", ] + select({ @@ -174,6 +184,7 @@ py_test( ":cel", ], deps = [ + ":leak_check", "//testing:proto2_test_all_types_py_pb2", "@com_google_absl_py//absl/testing:absltest", ] + select({ diff --git a/cel_expr_python/cel_parallel_test.py b/cel_expr_python/cel_parallel_test.py index 99100fd..1374f4f 100644 --- a/cel_expr_python/cel_parallel_test.py +++ b/cel_expr_python/cel_parallel_test.py @@ -25,6 +25,7 @@ from absl.testing import absltest from cel.expr.conformance.proto2 import test_all_types_pb2 as test_all_types_pb from cel_expr_python import cel +from cel_expr_python import leak_check @dataclasses.dataclass(frozen=True) @@ -94,6 +95,7 @@ class _TestCase: ] +@leak_check.leak_checked class CelParallelTest(absltest.TestCase): def setUp(self): @@ -119,7 +121,6 @@ def setUp(self): ) ], ) - self.object_counts_before_test = self._grab_object_counts() def tearDown(self): """Tears down the test environment.""" @@ -128,26 +129,6 @@ def tearDown(self): gc.collect() # Assert that all Arenas have been garbage-collected self.assertEqual(cel._InternalArena._get_instance_count(), 0) - self._check_for_leaks() - - def _grab_object_counts(self) -> dict[str, int]: - gc.collect() - all_objects = gc.get_objects() - type_counts = {} - for obj in all_objects: - obj_type = type(obj) - type_counts[obj_type.__name__] = type_counts.get(obj_type, 0) + 1 - return type_counts - - def _check_for_leaks(self): - type_counts = self._grab_object_counts() - for key, count in type_counts.items(): - if count != self.object_counts_before_test.get(key, 0): - self.fail( - f"Object count for {key} did not match expected count. " - f"Expected: {self.object_counts_before_test.get(key, 0)}, " - f"Actual: {count}", - ) def _test_eval(self, multi_threaded: bool): compiled_exprs = [self.env.compile(tc.expr) for tc in _TEST_CASES] diff --git a/cel_expr_python/cel_test.py b/cel_expr_python/cel_test.py index 2aca53c..be2df3d 100644 --- a/cel_expr_python/cel_test.py +++ b/cel_expr_python/cel_test.py @@ -26,12 +26,15 @@ from google.protobuf import duration_pb2 as duration_pb from google.protobuf import timestamp_pb2 as timestamp_pb from absl.testing import absltest +from absl.testing import parameterized from cel.expr.conformance.proto2 import test_all_types_pb2 as test_all_types_pb from cel_expr_python import cel +from cel_expr_python import leak_check +@leak_check.leak_checked @absltest.skipThisClass("Base class") -class _CelTestBase(absltest.TestCase): +class _CelTestBase(parameterized.TestCase): options: cel.Options = cel.Options() def setUp(self): @@ -57,7 +60,6 @@ def setUp(self): }, options=self.options, ) - self.object_counts_before_test = self._grab_object_counts() def tearDown(self): """Tears down the test environment.""" @@ -66,26 +68,6 @@ def tearDown(self): gc.collect() # Assert that all Arenas have been garbage-collected self.assertEqual(cel._InternalArena._get_instance_count(), 0) - self._check_for_leaks() - - def _grab_object_counts(self) -> dict[str, int]: - gc.collect() - all_objects = gc.get_objects() - type_counts = {} - for obj in all_objects: - obj_type = type(obj) - type_counts[obj_type.__name__] = type_counts.get(obj_type, 0) + 1 - return type_counts - - def _check_for_leaks(self): - type_counts = self._grab_object_counts() - for key, count in type_counts.items(): - if count != self.object_counts_before_test.get(key, 0): - self.fail( - f"Object count for {key} did not match expected count. " - f"Expected: {self.object_counts_before_test.get(key, 0)}, " - f"Actual: {count}", - ) def _eval( self, @@ -364,7 +346,46 @@ def testProto_unexpectedType(self): r" .*. \(Expected cel.expr.conformance.proto2.TestAllTypes\)", ) - def testProto_cannotFindMessageClass(self): + @parameterized.named_parameters( + ("message", "TestAllTypes{single_string: 'hello'}", True), + ("list", "[TestAllTypes{single_string: 'hello'}]", True), + ("nested_list", "[[1, 2], TestAllTypes{single_string: 'hello'}]", True), + ("map_value", "{'k': TestAllTypes{single_string: 'hello'}}", True), + ( + "nested_map_value", + ( + "{'a': [1, 2], 'b': [3, 4], 'z':" + " TestAllTypes{single_string: 'hello'}}" + ), + True, + ), + ("call_message", "show(TestAllTypes{single_string: 'hello'})", False), + ("call_list", "show([TestAllTypes{single_string: 'hello'}])", False), + ( + "call_nested_list", + "show([[1, 2], TestAllTypes{single_string: 'hello'}])", + False, + ), + ( + "call_map_value", + "show({'k': TestAllTypes{single_string: 'hello'}})", + False, + ), + ( + "call_nested_map_value", + ( + "show({'a': [1, 2], 'b': [3, 4], 'z':" + " TestAllTypes{single_string: 'hello'}})" + ), + False, + ), + ( + "call_second_arg", + "show([1, 2], TestAllTypes{single_string: 'hello'})", + False, + ), + ) + def testProto_cannotFindMessageClass(self, expr: str, expect_exception: bool): orig_get_message_class = message_factory.GetMessageClass def failing_get_message_class(descriptor): @@ -375,21 +396,78 @@ def failing_get_message_class(descriptor): try: message_factory.GetMessageClass = failing_get_message_class - env = cel.NewEnv(options=self.options) - expr = env.compile( - "cel.expr.conformance.proto2.TestAllTypes{single_string: 'hello'}" + called = [] + + def show(x): + called.append(x) + return "ok" + + def show_pair(a, b): + called.append((a, b)) + return "ok" + + ext = cel.CelExtension( + "show_ext", + functions=[ + cel.FunctionDecl( + "show", + [ + cel.Overload( + "show_any", cel.Type.STRING, [cel.Type.DYN], impl=show + ), + cel.Overload( + "show_pair", + cel.Type.STRING, + [cel.Type.DYN, cel.Type.DYN], + impl=show_pair, + ), + ], + ) + ], + ) + env = cel.NewEnv( + extensions=[ext], + options=self.options, + container="cel.expr.conformance.proto2", ) - res = expr.eval(env.Activation()) - with self.assertRaisesRegex( - TypeError, "Couldn't find message class for type" - ): - res.plain_value() - del res - del expr - del env + res = env.compile(expr).eval(env.Activation()) + if expect_exception: + with self.assertRaisesRegex( + TypeError, "Couldn't find message class for type" + ): + res.plain_value() + else: + # During evaluation of the function argument, the proto type will + # fail to resolve to a message class, and the error will be + # wrapped in an ERROR type. The function itself will not be called. + self.assertEqual(res.type(), cel.Type.ERROR) + self.assertIn("Couldn't find message class for type", res.value()) + self.assertEqual(called, []) finally: message_factory.GetMessageClass = orig_get_message_class + def testCustomFunction_raisesException(self): + def fail(x): + raise ValueError(f"bad value: {x}") + + ext = cel.CelExtension( + "fail_ext", + functions=[ + cel.FunctionDecl( + "fail", + [ + cel.Overload( + "fail_int", cel.Type.STRING, [cel.Type.INT], impl=fail + ) + ], + ) + ], + ) + env = cel.NewEnv(extensions=[ext], options=self.options) + res = env.compile("fail(42)").eval(env.Activation()) + self.assertEqual(res.type(), cel.Type.ERROR) + self.assertIn("bad value: 42", res.value()) + def testEvalList(self): res: cel.Value = self._eval( "[1, 'CEL', true]", expected_return_type=cel.Type.LIST diff --git a/cel_expr_python/leak_check.py b/cel_expr_python/leak_check.py new file mode 100644 index 0000000..b8c4ea5 --- /dev/null +++ b/cel_expr_python/leak_check.py @@ -0,0 +1,137 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Reference and memory leak detection helpers for CEL Python tests.""" + +import array +import collections.abc +import functools +import gc +import sys +import tracemalloc +from typing import Any, TypeVar + +_DEFAULT_WARMUPS = 2 +_DEFAULT_RUNS = 3 + +_F = TypeVar("_F", bound=collections.abc.Callable[..., Any]) +_T = TypeVar("_T", bound=type) + + +def _live_blocks() -> int: + """Returns the allocated Python memory block count after clearing caches.""" + # Clear CPython's internal caches (e.g., the type attribute lookup cache) + # before sampling so one-time cached entries are not mistaken for leaks. + clear_caches = getattr(sys, "_clear_internal_caches", None) or getattr( + sys, "_clear_type_cache", None + ) + if clear_caches is not None: + clear_caches() + gc.collect() + return sys.getallocatedblocks() + + +def _tracemalloc_report( + run_once: collections.abc.Callable[[], None], + iterations: int = _DEFAULT_RUNS, + limit: int = 5, +) -> str: + """Re-runs `run_once` under tracemalloc and formats top growing sites.""" + was_tracing = tracemalloc.is_tracing() + if not was_tracing: + tracemalloc.start(10) + try: + run_once() + _live_blocks() + before = tracemalloc.take_snapshot() + for _ in range(iterations): + run_once() + _live_blocks() + after = tracemalloc.take_snapshot() + finally: + if not was_tracing: + tracemalloc.stop() + ignore = [tracemalloc.Filter(False, "")] + if tracemalloc.__file__: + ignore.append(tracemalloc.Filter(False, tracemalloc.__file__)) + stats = after.filter_traces(ignore).compare_to( + before.filter_traces(ignore), "traceback" + ) + lines = [] + sites = 0 + for stat in stats: + if stat.count_diff <= 0: + continue + lines.append( + f" {stat.count_diff:+d} blocks ({stat.size_diff:+d} B) allocated at:" + ) + lines.extend( + " " + line for line in stat.traceback.format(most_recent_first=True) + ) + sites += 1 + if sites >= limit: + break + return "\n".join(lines) or " (no growth attributed by tracemalloc)" + + +def assert_no_leaks( + test_method: _F, + *, + warmups: int = _DEFAULT_WARMUPS, + runs: int = _DEFAULT_RUNS, +) -> _F: + """Wraps a `unittest.TestCase` method to fail if live blocks grow every run. + + Uses CPython's `regrtest -R` strategy: runs `warmups` unmeasured iterations + to populate lazy process-wide caches, then `runs` measured iterations, + failing only if `sys.getallocatedblocks()` increases on every measured run. + + Args: + test_method: The test method to wrap. + warmups: Number of warm-up repetitions before measuring. + runs: Number of measured repetitions. + + Returns: + The wrapped test method. + """ + + @functools.wraps(test_method) + def wrapper(self: Any, *args: Any, **kwargs: Any) -> Any: + total_runs = warmups + runs + # Store samples in a C array so recording them keeps no Python ints alive. + blocks = array.array("q", [0]) * (total_runs + 1) + blocks[0] = _live_blocks() + for i in range(total_runs): + test_method(self, *args, **kwargs) + blocks[i + 1] = _live_blocks() + deltas = [blocks[i + 1] - blocks[i] for i in range(warmups, total_runs)] + if all(d > 0 for d in deltas): + self.fail( + f"Memory blocks grew on every one of {runs} repetitions" + f" (after {warmups} warm-up runs): deltas={deltas}." + " Top growing allocation sites:\n" + + _tracemalloc_report( + lambda: test_method(self, *args, **kwargs), iterations=runs + ) + ) + + return wrapper # type: ignore[return-value] + + +def leak_checked(cls: _T) -> _T: + """Class decorator wrapping every `test*` method with `assert_no_leaks`.""" + for name, attr in list(cls.__dict__.items()): + if name.startswith("test") and callable(attr): + setattr(cls, name, assert_no_leaks(attr)) + return cls diff --git a/cel_expr_python/py_cel_function.cc b/cel_expr_python/py_cel_function.cc index f42d38c..dfd8387 100644 --- a/cel_expr_python/py_cel_function.cc +++ b/cel_expr_python/py_cel_function.cc @@ -16,6 +16,7 @@ #include +#include #include #include #include @@ -107,14 +108,34 @@ absl::StatusOr PyCelFunctionAdapter::Invoke( CEL_PYTHON_ASSIGN_OR_RETURN(auto py_arena, PyCelArena::FromProtoArena(context.arena())); PyObject* py_args = PyTuple_New(args.size()); - for (int i = 0; i < args.size(); ++i) { - PyTuple_SetItem(py_args, i, - CelValueToPyObject(args[i], env, py_arena, - /*plain_value=*/true)); + if (py_args == nullptr) { + absl::Status status = PyErr_toStatus(); + if (status.ok()) { + status = absl::ResourceExhaustedError("Failed to allocate args tuple"); + } + return cel::ErrorValue::From(status, context.arena()); + } + for (size_t i = 0; i < args.size(); ++i) { + PyObject* py_arg = CelValueToPyObject(args[i], env, py_arena, + /*plain_value=*/true); + if (py_arg == nullptr) { + absl::Status status = PyErr_toStatus(); + Py_DECREF(py_args); + if (status.ok()) { + status = absl::InternalError( + "Failed to convert CEL argument to Python object"); + } + return cel::ErrorValue::From(status, context.arena()); + } + PyTuple_SetItem(py_args, i, py_arg); } PyObject* result = PyObject_CallObject(py_function_, py_args); - Py_DECREF(py_args); absl::Status status = PyErr_toStatus(); + Py_DECREF(py_args); + if (status.ok() && result == nullptr) { + status = absl::InternalError( + "Python function returned null without setting an error"); + } if (!status.ok()) { Py_XDECREF(result); return cel::ErrorValue::From(status, context.arena()); diff --git a/cel_expr_python/py_cel_value.cc b/cel_expr_python/py_cel_value.cc index ca13f77..9ed238f 100644 --- a/cel_expr_python/py_cel_value.cc +++ b/cel_expr_python/py_cel_value.cc @@ -55,6 +55,24 @@ namespace cel_python { namespace py = ::pybind11; +namespace { + +std::string PyObjectRepr(PyObject* obj) { + if (obj == nullptr) { + return ""; + } + PyObject* repr = PyObject_Repr(obj); + const char* repr_cstr = repr != nullptr ? PyUnicode_AsUTF8(repr) : nullptr; + if (repr_cstr == nullptr) { + PyErr_Clear(); + } + std::string repr_str = repr_cstr != nullptr ? repr_cstr : ""; + Py_XDECREF(repr); + return repr_str; +} + +} // namespace + void PyCelValue::DefinePythonBindings(py::module& m) { py::class_(m, "Value") .def("type", &PyCelValue::Type) @@ -461,8 +479,11 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, cel::ListValue list = *cel_value.AsList(); Py_ssize_t size = *list.Size(); PyObject* py_list = PyList_New(size); + if (py_list == nullptr) { + return nullptr; + } absl::Status status = absl::OkStatus(); - for (int i = 0; i < size; ++i) { + for (Py_ssize_t i = 0; i < size; ++i) { PyObject* py_item; if (plain_value) { absl::StatusOr item = @@ -479,6 +500,10 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, .release() .ptr(); } + if (py_item == nullptr) { + Py_DECREF(py_list); + return nullptr; + } // PyList_SetItem is special in that it "steals" the reference to the // item being set, so we must not decref it. // https://docs.python.org/3/c-api/list.html#:~:text=int-,pylist_setitem @@ -505,12 +530,16 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, } PyObject* py_map = PyDict_New(); + if (py_map == nullptr) { + return nullptr; + } Py_ssize_t size = *key_list.Size(); - for (int i = 0; i < size; ++i) { + for (Py_ssize_t i = 0; i < size; ++i) { absl::StatusOr key = key_list.Get(i, env->GetDescriptorPool(), env->GetMessageFactory(), arena->GetArena()); if (!key.ok()) { + Py_DECREF(py_map); PyErr_Format(PyExc_RuntimeError, "Cannot get key: %s", key.status().ToString().c_str()); return nullptr; @@ -518,6 +547,10 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, PyObject* py_key = CelValueToPyObject(*key, env, arena, /*plain_value=*/true); + if (py_key == nullptr) { + Py_DECREF(py_map); + return nullptr; + } PyObject* py_value; if (plain_value) { cel::Value element_value; @@ -525,9 +558,10 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, map.Get(*key, env->GetDescriptorPool(), env->GetMessageFactory(), arena->GetArena(), &element_value); if (!status.ok()) { + Py_DECREF(py_key); + Py_DECREF(py_map); PyErr_Format(PyExc_RuntimeError, "Cannot get value: %s", status.ToString().c_str()); - Py_DECREF(py_key); return nullptr; } py_value = CelValueToPyObject(element_value, env, arena, @@ -537,12 +571,21 @@ PyObject* CelValueToPyObject(const cel::Value& cel_value, .release() .ptr(); } + if (py_value == nullptr) { + Py_DECREF(py_key); + Py_DECREF(py_map); + return nullptr; + } // In contrast to PyList_SetItem, PyDict_SetItem does not "steal" the // reference to the key and value being set, so we must decref them. // https://docs.python.org/3/c-api/dict.html#:~:text=int-,pydict_setitem - PyDict_SetItem(py_map, py_key, py_value); + int set_status = PyDict_SetItem(py_map, py_key, py_value); Py_DECREF(py_key); Py_DECREF(py_value); + if (set_status < 0) { + Py_DECREF(py_map); + return nullptr; + } } return py_map; } @@ -767,9 +810,9 @@ absl::StatusOr PyObjectToCelValue( time = py::cast(py_object); } catch (const py::cast_error& e) { PyErr_Clear(); - return absl::UnknownError(absl::StrFormat( - "Cannot convert %s to absl::Time: %s", - PyUnicode_AsUTF8(PyObject_Repr(py_object)), e.what())); + return absl::UnknownError( + absl::StrFormat("Cannot convert %s to absl::Time: %s", + PyObjectRepr(py_object), e.what())); } absl::StatusOr timestamp_value = cel::SafeTimestampValue(time); @@ -808,7 +851,7 @@ absl::StatusOr PyObjectToCelValue( PyErr_Clear(); return absl::UnknownError( absl::StrFormat("Cannot convert %s to cel.Type.TIMESTAMP", - PyUnicode_AsUTF8(PyObject_Repr(py_object)))); + PyObjectRepr(py_object))); } absl::Time time = absl::FromUnixSeconds(seconds) + absl::Nanoseconds(nanos); @@ -830,9 +873,9 @@ absl::StatusOr PyObjectToCelValue( duration = py::cast(py_object); } catch (const py::cast_error& e) { PyErr_Clear(); - return absl::UnknownError(absl::StrFormat( - "Cannot convert %s to absl::Duration: %s", - PyUnicode_AsUTF8(PyObject_Repr(py_object)), e.what())); + return absl::UnknownError( + absl::StrFormat("Cannot convert %s to absl::Duration: %s", + PyObjectRepr(py_object), e.what())); } absl::StatusOr duration_value = cel::SafeDurationValue(duration); @@ -869,9 +912,8 @@ absl::StatusOr PyObjectToCelValue( Py_XDECREF(py_nanos); if (!ok) { PyErr_Clear(); - return absl::UnknownError( - absl::StrFormat("Cannot convert %s to absl::Duration", - PyUnicode_AsUTF8(PyObject_Repr(py_object)))); + return absl::UnknownError(absl::StrFormat( + "Cannot convert %s to absl::Duration", PyObjectRepr(py_object))); } absl::Duration duration = absl::Seconds(seconds) + absl::Nanoseconds(nanos); @@ -953,28 +995,26 @@ absl::StatusOr PyObjectToCelValue( while (PyDict_Next(py_object, &pos, &key, &value)) { // Note: PyDict_Next returns borrowed references, so we shouldn't // DECREF them. - CEL_PYTHON_ASSIGN_OR_RETURN( - cel::Value converted_key, - PyObjectToCelValue( - key, key_type, - [context, key]() { - return absl::StrFormat( - "%s[%s]", context(), - PyUnicode_AsUTF8(PyObject_Repr(key))); - }, - env, arena)); - - CEL_PYTHON_ASSIGN_OR_RETURN( - cel::Value converted_value, - PyObjectToCelValue( - value, value_type, - [context, key, value]() { - return absl::StrFormat( - "%s[%s] -> %s", context(), - PyUnicode_AsUTF8(PyObject_Repr(key)), - PyUnicode_AsUTF8(PyObject_Repr(value))); - }, - env, arena)); + CEL_PYTHON_ASSIGN_OR_RETURN(cel::Value converted_key, + PyObjectToCelValue( + key, key_type, + [context, key]() { + return absl::StrFormat( + "%s[%s]", context(), + PyObjectRepr(key)); + }, + env, arena)); + + CEL_PYTHON_ASSIGN_OR_RETURN(cel::Value converted_value, + PyObjectToCelValue( + value, value_type, + [context, key, value]() { + return absl::StrFormat( + "%s[%s] -> %s", context(), + PyObjectRepr(key), + PyObjectRepr(value)); + }, + env, arena)); CEL_PYTHON_RETURN_IF_ERROR( builder->Put(converted_key, converted_value)); } diff --git a/cel_expr_python/py_message_factory.cc b/cel_expr_python/py_message_factory.cc index 080b33b..b2d96f0 100644 --- a/cel_expr_python/py_message_factory.cc +++ b/cel_expr_python/py_message_factory.cc @@ -43,11 +43,11 @@ PyMessageFactory::PyMessageFactory(PyObject* descriptor_pool) { ABSL_LOG(FATAL) << "Cannot load module 'google.protobuf.message_factory'"; } py_func_GetMessageClass_ = PyObject_GetAttrString(pModule, "GetMessageClass"); + Py_DECREF(pModule); if (!py_func_GetMessageClass_) { ABSL_LOG(FATAL) << "Cannot find function " "'google.protobuf.message_factory.GetMessageClass'"; } - Py_INCREF(py_func_GetMessageClass_); py_func_MergeFromString_ = PyUnicode_FromString("MergeFromString"); } @@ -146,12 +146,12 @@ PyObject* PyMessageFactory::FromString(const std::string& message_type, PyObject* ret = PyObject_CallMethodObjArgs(message, py_func_MergeFromString_, serialized_proto_py, nullptr); + Py_DECREF(serialized_proto_py); if (!ret) { Py_DECREF(message); - Py_DECREF(serialized_proto_py); return nullptr; } - Py_DECREF(serialized_proto_py); + Py_DECREF(ret); return message; } diff --git a/release/build_wheel.sh b/release/build_wheel.sh index eeeef3f..6b45fb8 100755 --- a/release/build_wheel.sh +++ b/release/build_wheel.sh @@ -69,7 +69,7 @@ if [[ "$OSTYPE" == "darwin"* ]]; then else cp -r "${SRC_DIR}/." . fi -rm -rf cel_expr_python/*_test.py +rm -rf cel_expr_python/*_test.py cel_expr_python/leak_check.py PYTHON_BIN="python" if command -v python3 &> /dev/null; then diff --git a/release/cel_basic_test.py b/release/cel_basic_test.py index 7295243..902abea 100644 --- a/release/cel_basic_test.py +++ b/release/cel_basic_test.py @@ -192,6 +192,7 @@ def test_wheel_package_contents(self): ("cel_expr_python/", "cel_expr_python-", "cel_expr_python.libs/") ) or f.endswith("_test.py") + or f == "cel_expr_python/leak_check.py" or "/testdata/" in f ] self.assertEqual( diff --git a/release/kokoro/release_linux.sh b/release/kokoro/release_linux.sh index 08bfe0c..3ff4c95 100755 --- a/release/kokoro/release_linux.sh +++ b/release/kokoro/release_linux.sh @@ -243,7 +243,7 @@ pushd "${TMP_DIR}" cp -r "${SRC_DIR}"/{*,.*} . 2>/dev/null || true cp "${SRC_DIR}"/release/*.py "${SRC_DIR}"/release/*.toml . 2>/dev/null || true -rm -rf cel_expr_python/*_test.py +rm -rf cel_expr_python/*_test.py cel_expr_python/leak_check.py echo "Downloading bazelisk on host..." curl -LO https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-amd64 diff --git a/release/kokoro/release_macos.sh b/release/kokoro/release_macos.sh index e36b555..52c1dd3 100644 --- a/release/kokoro/release_macos.sh +++ b/release/kokoro/release_macos.sh @@ -58,7 +58,7 @@ pushd "${TMP_DIR}" cp -r "${SRC_DIR}"/{*,.*} . 2>/dev/null || true cp "${SRC_DIR}"/release/*.py "${SRC_DIR}"/release/*.toml . 2>/dev/null || true -rm -rf cel_expr_python/*_test.py +rm -rf cel_expr_python/*_test.py cel_expr_python/leak_check.py # Check if pyproject.toml exists before running sed if [ -f pyproject.toml ]; then diff --git a/release/kokoro/release_windows.bat b/release/kokoro/release_windows.bat index 2931936..f0d7a37 100644 --- a/release/kokoro/release_windows.bat +++ b/release/kokoro/release_windows.bat @@ -131,6 +131,9 @@ if !ERRORLEVEL! NEQ 0 ( if exist "cel_expr_python\*_test.py" ( del /Q "cel_expr_python\*_test.py" ) +if exist "cel_expr_python\leak_check.py" ( + del /Q "cel_expr_python\leak_check.py" +) :: Substitute $VERSION in pyproject.toml with the value of VERSION. !PYTHON_EXE! -c "import sys; content = open('pyproject.toml').read(); open('pyproject.toml', 'w').write(content.replace('$VERSION', sys.argv[1]))" "%VERSION%" diff --git a/release/pyproject.toml b/release/pyproject.toml index 3b54d9e..f704343 100644 --- a/release/pyproject.toml +++ b/release/pyproject.toml @@ -42,7 +42,7 @@ exclude = ["*testdata*"] "*" = ["*.pyi"] [tool.setuptools.exclude-package-data] -"*" = ["*_test.py"] +"*" = ["*_test.py", "leak_check.py"] [tool.cibuildwheel] build = "cp310-* cp311-* cp312-* cp313-* cp314-*"