CLIENT-3915 suppression improvements #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft

DomPeliniAerospike wants to merge 16 commits into dev from CLIENT-3915-Suppression-Improvements

.github/workflows/valgrind.yml

-Original file line number
+Diff line change
@@ Expand Up / @@ -166,13 +166,44 @@ jobs: @@
         - run: echo VALGRIND_ARGS="--tool=massif --massif-out-file=./${{ env.MASSIF_REPORT_FILE_NAME }}" >> $GITHUB_ENV
           if: ${{ inputs.massif }}
-        - run: echo VALGRIND_ARGS="--leak-check=full" >> $GITHUB_ENV
+        #- run: echo VALGRIND_ARGS="--leak-check=full --gen-suppressions=all --suppressions=./valgrind-suppression/new_tests.supp --suppressions=./valgrind-suppression/file-suppressions/${{ github.event.inputs.test-file }}.supp --num-callers=350 --error-limit=no" >> $GITHUB_ENV
+        #  if: ${{ !inputs.massif && github.event.inputs.test-file != '' }}
+        - run: echo VALGRIND_ARGS="--leak-check=full --gen-suppressions=all --suppressions=./valgrind-suppression/new_tests.supp --num-callers=350 --error-limit=no" >> $GITHUB_ENV
+          if: ${{ !inputs.massif && github.event.inputs.test-file != '' }}
+        - run: echo VALGRIND_ARGS="--leak-check=full --gen-suppressions=all --suppressions=./valgrind-suppression/new_tests.supp --suppressions=./valgrind-suppression/file-suppressions/${{ github.event.inputs.test-file }}.supp --num-callers=350 --error-limit=no " >> $GITHUB_ENV
+          if: ${{ !inputs.massif && github.event.inputs.test-file == '' }}
+        - run: PYTHONMALLOC=malloc valgrind --error-exitcode=1 ${{ env.VALGRIND_ARGS }} python3 -m pytest -v new_tests/${{ github.event.inputs.test-file }} 2>&1 | tee valgrind.log
+          working-directory: test
+        - run: |
+            python3 valgrind-suppression/extract_suppressions.py valgrind.log {{ github.event.inputs.test-file }}.supp
+            cat {{ github.event.inputs.test-file }}.supp
+          working-directory: test
           if: ${{ !inputs.massif }}
-        - run: PYTHONMALLOC=malloc valgrind --error-exitcode=1 ${{ env.VALGRIND_ARGS }} python3 -m pytest -v new_tests/${{ github.event.inputs.test-file }}
+        - name: Upload suppression file
+          uses: actions/upload-artifact@v4
+          with:
+            name: ${{ github.event.inputs.test-file }}.supp
+            path: test/{{ github.event.inputs.test-file }}.supp
+          if: ${{ !inputs.massif }}
+        - run: |
+            python3 valgrind-suppression/filter_suppressions.py new_suppressions.supp filtered_{{ github.event.inputs.test-file }}.supp
+            cat filtered_{{ github.event.inputs.test-file }}.supp
           working-directory: test
+          if: ${{ !inputs.massif }}
+        - name: Upload filter suppressions file
+          uses: actions/upload-artifact@v4
+          with:
+            name: filtered_${{ github.event.inputs.test-file }}.supp
+            path: test/filtered_{{ github.event.inputs.test-file }}.supp
+          if: ${{ !inputs.massif }}
-        # TODO: upload report as artifact
         - run: ms_print ./${{ env.MASSIF_REPORT_FILE_NAME }}
           if: ${{ !cancelled() && inputs.massif }}
           working-directory: test
@@ Expand Down @@

src/main/conversions.c

-Original file line number
+Diff line change
@@ Expand Up / @@ -406,6 +406,8 @@ as_status as_partitions_status_to_pyobject( @@
                 Py_XDECREF(py_id);
                 goto END;
             }
+            // Must be decref'd because PyDict_SetItem does not steal a reference
+            Py_DECREF(new_py_tuple);
             Py_DECREF(py_id);
         }
@@ Expand Down Expand Up @@
             Py_DECREF(py_parameter);
             PyObject *geospatial_dump = AerospikeGeospatial_DoDumps(py_data, err);
+            Py_XDECREF(py_data);
+            if (!geospatial_dump) {
+                PyErr_Clear();
+                return as_error_update(err, AEROSPIKE_ERR_CLIENT,
+                                       "Unable to call dumps function");
+            }
             const char *geo_value = PyUnicode_AsUTF8(geospatial_dump);
             char *geo_value_cpy = strdup(geo_value);
-            Py_DECREF(py_data);
             Py_DECREF(geospatial_dump);
             *val = (as_val *)as_geojson_new(geo_value_cpy, true);
@@ Expand Down Expand Up @@
         else if (!strcmp(py_value->ob_type->tp_name, "aerospike.Geospatial")) {
             PyObject *geo_data = PyObject_GetAttrString(py_value, "geo_data");
             PyObject *geo_data_py_str = AerospikeGeospatial_DoDumps(geo_data, err);
+            Py_XDECREF(geo_data);
+            if (!geo_data_py_str) {
+                PyErr_Clear();
+                as_error_update(err, AEROSPIKE_ERR_CLIENT,
+                                "Unable to call dumps function");
+                return;
+            }
             const char *geo_data_str = PyUnicode_AsUTF8(geo_data_py_str);
             // Make a copy of the encoding since the utf8 encoding points to a buffer in the PyUnicode object
@@ Expand All @@
             // Cleanup
             Py_XDECREF(geo_data_py_str);
-            Py_XDECREF(geo_data);
         }
         else if (!strcmp(py_value->ob_type->tp_name, "aerospike.null")) {
             ((as_val *)&binop_bin->value)->type = AS_UNKNOWN;
@@ Expand Down @@

src/main/convert_expressions.c

-Original file line number
+Diff line change
@@ Expand Up @@
             PyObject *py_parameter = PyUnicode_FromString("geo_data");
             PyObject *py_data = PyObject_GenericGetAttr(py_obj, py_parameter);
             Py_DECREF(py_parameter);
-            char *geo_value =
-                (char *)PyUnicode_AsUTF8(AerospikeGeospatial_DoDumps(py_data, err));
-            Py_DECREF(py_data);
+            PyObject *geospatial_dump = AerospikeGeospatial_DoDumps(py_data, err);
+            Py_XDECREF(py_data);
+            if (!geospatial_dump) {
+                PyErr_Clear();
+                return as_error_update(err, AEROSPIKE_ERR_CLIENT,
+                                       "Unable to call dumps function");
+            }
+            char *geo_value = (char *)PyUnicode_AsUTF8(geospatial_dump);
             as_exp_entry tmp_entry = as_exp_geo(geo_value);
             *new_entry = tmp_entry;
         }
@@ Expand Down @@

src/main/geospatial/dumps.c

-Original file line number
+Diff line change
@@ Expand Up @@
             json_module = PyImport_ImportModule("json");
         }
-        if (!json_module) {
-            /* insert error handling here! and exit this function */
-            as_error_update(err, AEROSPIKE_ERR_CLIENT,
-                            "Unable to load json module");
-        }
-        else {
+        if (json_module && geo_data) {
             PyObject *py_funcname = PyUnicode_FromString("dumps");
             initresult = PyObject_CallMethodObjArgs(json_module, py_funcname,
                                                     geo_data, NULL);
@@ Expand Down Expand Up @@
         initresult = AerospikeGeospatial_DoDumps(self->geo_data, &err);
         if (!initresult) {
+            PyErr_Clear();
             as_error_update(&err, AEROSPIKE_ERR_CLIENT,
                             "Unable to call dumps function");
             goto CLEANUP;
@@ Expand Down @@

src/main/predicates.c

-Original file line number
+Diff line change
@@ Expand Up @@
             py_shape = AerospikeGeospatial_DoDumps(py_geo_object, &err);
             if (!py_shape) {
+                PyErr_Clear();
                 as_error_update(&err, AEROSPIKE_ERR_CLIENT,
                                 "Unable to call dumps function");
                 goto CLEANUP;
@@ Expand All @@
                             "double type, bin of string type");
             goto CLEANUP;
         }
-        Py_CLEAR(py_geo_object);
         ret_val = Py_BuildValue("iiOOOO", AS_PREDICATE_RANGE, AS_INDEX_GEO2DSPHERE,
                                 py_bin, py_shape, Py_None, py_indexType);
@@ Expand Down Expand Up @@
             py_list = Py_BuildValue("[OO]", py_lat, py_long);
             PyDict_SetItemString(py_geo_object, "coordinates", py_list);
-            Py_CLEAR(py_list);
             py_shape = AerospikeGeospatial_DoDumps(py_geo_object, &err);
-            Py_CLEAR(py_geo_object);
             if (!py_shape) {
+                PyErr_Clear();
                 as_error_update(&err, AEROSPIKE_ERR_CLIENT,
                                 "Unable to call dumps function");
                 goto CLEANUP;
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLIENT-3915 suppression improvements #881

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

CLIENT-3915 suppression improvements #881

Are you sure you want to change the base?

Uh oh!

CLIENT-3915 suppression improvements #881

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!