PYTHON-3222 Fix memory leak in cbson decode_all (#927)

Add decode_all keyword arg for codec_options.
Make decode_all show up in docs.

(cherry picked from commit dca72b7)

Author: ShaneHarvey

bson/__init__.py

@@ -965,38 +965,8 @@ def decode(data, codec_options=DEFAULT_CODEC_OPTIONS):
     return _bson_to_dict(data, codec_options)
 
 
-def decode_all(data, codec_options=DEFAULT_CODEC_OPTIONS):
-    """Decode BSON data to multiple documents.
-
-    `data` must be a bytes-like object implementing the buffer protocol that
-    provides concatenated, valid, BSON-encoded documents.
-
-    :Parameters:
-      - `data`: BSON data
-      - `codec_options` (optional): An instance of
-        :class:`~bson.codec_options.CodecOptions`.
-
-    .. versionchanged:: 3.9
-       Supports bytes-like objects that implement the buffer protocol.
-
-    .. versionchanged:: 3.0
-       Removed `compile_re` option: PyMongo now always represents BSON regular
-       expressions as :class:`~bson.regex.Regex` objects. Use
-       :meth:`~bson.regex.Regex.try_compile` to attempt to convert from a
-       BSON regular expression to a Python regular expression object.
-
-       Replaced `as_class`, `tz_aware`, and `uuid_subtype` options with
-       `codec_options`.
-
-    .. versionchanged:: 2.7
-       Added `compile_re` option. If set to False, PyMongo represented BSON
-       regular expressions as :class:`~bson.regex.Regex` objects instead of
-       attempting to compile BSON regular expressions as Python native
-       regular expressions, thus preventing errors for some incompatible
-       patterns, see `PYTHON-500`_.
-
-    .. _PYTHON-500: https://jira.mongodb.org/browse/PYTHON-500
-    """
+def _decode_all(data, codec_options):
+    """Decode BSON data to multiple documents."""
     data, view = get_data_and_view(data)
     if not isinstance(codec_options, CodecOptions):
         raise _CODEC_OPTIONS_TYPE_ERROR
@@ -1031,7 +1001,44 @@ def decode_all(data, codec_options=DEFAULT_CODEC_OPTIONS):
 
 
 if _USE_C:
-    decode_all = _cbson.decode_all
+    _decode_all = _cbson._decode_all
+
+
+def decode_all(data, codec_options=DEFAULT_CODEC_OPTIONS):
+    """Decode BSON data to multiple documents.
+
+    `data` must be a bytes-like object implementing the buffer protocol that
+    provides concatenated, valid, BSON-encoded documents.
+
+    :Parameters:
+      - `data`: BSON data
+      - `codec_options` (optional): An instance of
+        :class:`~bson.codec_options.CodecOptions`.
+
+    .. versionchanged:: 3.9
+       Supports bytes-like objects that implement the buffer protocol.
+
+    .. versionchanged:: 3.0
+       Removed `compile_re` option: PyMongo now always represents BSON regular
+       expressions as :class:`~bson.regex.Regex` objects. Use
+       :meth:`~bson.regex.Regex.try_compile` to attempt to convert from a
+       BSON regular expression to a Python regular expression object.
+
+       Replaced `as_class`, `tz_aware`, and `uuid_subtype` options with
+       `codec_options`.
+
+    .. versionchanged:: 2.7
+       Added `compile_re` option. If set to False, PyMongo represented BSON
+       regular expressions as :class:`~bson.regex.Regex` objects instead of
+       attempting to compile BSON regular expressions as Python native
+       regular expressions, thus preventing errors for some incompatible
+       patterns, see `PYTHON-500`_.
+
+    .. _PYTHON-500: https://jira.mongodb.org/browse/PYTHON-500
+    """
+    if not isinstance(codec_options, CodecOptions):
+        raise _CODEC_OPTIONS_TYPE_ERROR
+    return _decode_all(data, codec_options)
 
 
 def _decode_selective(rawdoc, fields, codec_options):

bson/_cbsonmodule.c

@@ -53,7 +53,6 @@ struct module_state {
     PyObject* BSONInt64;
     PyObject* Decimal128;
     PyObject* Mapping;
-    PyObject* CodecOptions;
 };
 
 /* The Py_TYPE macro was introduced in CPython 2.6 */
@@ -371,11 +370,10 @@ static int _load_python_objects(PyObject* module) {
         _load_object(&state->Decimal128, "bson.decimal128", "Decimal128") ||
         _load_object(&state->UUID, "uuid", "UUID") ||
 #if PY_MAJOR_VERSION >= 3
-        _load_object(&state->Mapping, "collections.abc", "Mapping") ||
+        _load_object(&state->Mapping, "collections.abc", "Mapping")) {
 #else
-        _load_object(&state->Mapping, "collections", "Mapping") ||
+        _load_object(&state->Mapping, "collections", "Mapping")) {
 #endif
-        _load_object(&state->CodecOptions, "bson.codec_options", "CodecOptions")) {
         return 1;
     }
     /* Reload our REType hack too. */
@@ -537,26 +535,6 @@ int convert_codec_options(PyObject* options_obj, void* p) {
     return 1;
 }
 
-/* Fill out a codec_options_t* with default options.
- *
- * Return 1 on success.
- * Return 0 on failure.
- */
-int default_codec_options(struct module_state* state, codec_options_t* options) {
-    PyObject* options_obj = NULL;
-    PyObject* codec_options_func = _get_object(
-        state->CodecOptions, "bson.codec_options", "CodecOptions");
-    if (codec_options_func == NULL) {
-        return 0;
-    }
-    options_obj = PyObject_CallFunctionObjArgs(codec_options_func, NULL);
-    Py_DECREF(codec_options_func);
-    if (options_obj == NULL) {
-        return 0;
-    }
-    return convert_codec_options(options_obj, options);
-}
-
 void destroy_codec_options(codec_options_t* options) {
     Py_CLEAR(options->document_class);
     Py_CLEAR(options->tzinfo);
@@ -2675,15 +2653,10 @@ static PyObject* _cbson_element_to_dict(PyObject* self, PyObject* args) {
     PyObject* value;
     PyObject* result_tuple;
 
-    if (!PyArg_ParseTuple(args, "OII|O&", &bson, &position, &max,
+    if (!PyArg_ParseTuple(args, "OIIO&", &bson, &position, &max,
                           convert_codec_options, &options)) {
         return NULL;
     }
-    if (PyTuple_GET_SIZE(args) < 4) {
-        if (!default_codec_options(GETSTATE(self), &options)) {
-            return NULL;
-        }
-    }
 
 #if PY_MAJOR_VERSION >= 3
     if (!PyBytes_Check(bson)) {
@@ -2867,17 +2840,13 @@ static PyObject* _cbson_decode_all(PyObject* self, PyObject* args) {
     PyObject* dict;
     PyObject* result = NULL;
     codec_options_t options;
-    PyObject* options_obj;
-    Py_buffer view;
+    PyObject* options_obj = NULL;
+    Py_buffer view = {0};
 
-    if (!PyArg_ParseTuple(args, "O|O", &bson, &options_obj)) {
+    if (!PyArg_ParseTuple(args, "OO", &bson, &options_obj)) {
         return NULL;
     }
-    if (PyTuple_GET_SIZE(args) < 2) {
-        if (!default_codec_options(GETSTATE(self), &options)) {
-            return NULL;
-        }
-    } else if (!convert_codec_options(options_obj, &options)) {
+    if (!convert_codec_options(options_obj, &options)) {
         return NULL;
     }
 
@@ -2971,7 +2940,7 @@ static PyMethodDef _CBSONMethods[] = {
      "convert a dictionary to a string containing its BSON representation."},
     {"_bson_to_dict", _cbson_bson_to_dict, METH_VARARGS,
      "convert a BSON string to a SON object."},
-    {"decode_all", _cbson_decode_all, METH_VARARGS,
+    {"_decode_all", _cbson_decode_all, METH_VARARGS,
      "convert binary data to a sequence of documents."},
     {"_element_to_dict", _cbson_element_to_dict, METH_VARARGS,
      "Decode a single key, value pair."},

doc/changelog.rst

@@ -14,6 +14,10 @@ Bug fixes
 
 - Fixed a bug where the client could be unable to discover the new primary
   after a simultaneous replica set election and reconfig (`PYTHON-2970`_).
+- Fixed a memory leak bug when calling :func:`~bson.decode_all` without a
+  ``codec_options`` argument (`PYTHON-3222`_).
+- Fixed a bug where :func:`~bson.decode_all` did not accept ``codec_options``
+  as a keyword argument (`PYTHON-3222`_).
 
 Deprecations
 ............
@@ -26,6 +30,7 @@ See the `PyMongo 3.13.0 release notes in JIRA`_ for the list of resolved issues
 in this release.
 
 .. _PYTHON-2970: https://jira.mongodb.org/browse/PYTHON-2970
+.. _PYTHON-3222: https://jira.mongodb.org/browse/PYTHON-3222
 .. _PyMongo 3.13.0 release notes in JIRA: https://jira.mongodb.org/secure/ReleaseNote.jspa?projectId=10004&version=31570
 
 Changes in Version 3.12.3

test/test_bson.py

@@ -41,7 +41,7 @@
     encode,
     is_valid,
 )
-from bson.binary import Binary, UUIDLegacy
+from bson.binary import Binary, UUIDLegacy, UuidRepresentation
 from bson.code import Code
 from bson.codec_options import CodecOptions
 from bson.dbref import DBRef
@@ -1036,6 +1036,15 @@ def test_decode_all_defaults(self):
         self.assertEqual(decoded["uuid"], doc["uuid"])
         self.assertIsNone(decoded["dt"].tzinfo)
 
+    def test_decode_all_kwarg(self):
+        doc = {"a": uuid.uuid4()}
+        opts = CodecOptions(uuid_representation=UuidRepresentation.STANDARD)
+        encoded = encode(doc, codec_options=opts)
+        # Positional codec_options
+        self.assertEqual([doc], decode_all(encoded, opts))
+        # Keyword codec_options
+        self.assertEqual([doc], decode_all(encoded, codec_options=opts))
+
     def test_unicode_decode_error_handler(self):
         enc = encode({"keystr": "foobar"})