[arrow] branch main updated: GH-35760: [C++] C Data Interface helpers should also run checks in non-debug mode (#36215)

[ap...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new de4936d8eb GH-35760: [C++] C Data Interface helpers should also run checks in non-debug mode (#36215)
de4936d8eb is described below

commit de4936d8eb7f639fd268637195bd8741498860c9
Author: Matt Topol <zo...@gmail.com>
AuthorDate: Tue Jun 27 14:05:11 2023 -0400

    GH-35760: [C++] C Data Interface helpers should also run checks in non-debug mode (#36215)
    
    
    
    ### What changes are included in this PR?
    Replaced `assert`s in the `Arrow*Release` methods with a macro which outputs to `stderr` and calls `std::abort` directly with a message. This way the check still happens in release mode. The `ARROW_CHECK*` macros require C++ and this is intended to be a C file which others can vendor.
    
    ### Are there any user-facing changes?
    Instead of only failing in debug mode, now these release methods will fail if the release callback fails to set the `release` member to `null` even in release mode. Leading to potential calls to `std::abort` that weren't previously occurring.
    
    * Closes: #35760
    
    Lead-authored-by: Matt Topol <zo...@gmail.com>
    Co-authored-by: Antoine Pitrou <an...@python.org>
    Co-authored-by: Antoine Pitrou <pi...@free.fr>
    Signed-off-by: Antoine Pitrou <an...@python.org>
---
 cpp/src/arrow/c/helpers.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/cpp/src/arrow/c/helpers.h b/cpp/src/arrow/c/helpers.h
index a5c1f6fe4b..a24f272fea 100644
--- a/cpp/src/arrow/c/helpers.h
+++ b/cpp/src/arrow/c/helpers.h
@@ -17,11 +17,20 @@
 
 #pragma once
 
-#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include "arrow/c/abi.h"
 
+#define ARROW_C_ASSERT(condition, msg)                          \
+  do {                                                          \
+    if (!(condition)) {                                         \
+      fprintf(stderr, "%s:%d:: %s", __FILE__, __LINE__, (msg)); \
+      abort();                                                  \
+    }                                                           \
+  } while (0)
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -51,7 +60,8 @@ inline void ArrowSchemaMove(struct ArrowSchema* src, struct ArrowSchema* dest) {
 inline void ArrowSchemaRelease(struct ArrowSchema* schema) {
   if (!ArrowSchemaIsReleased(schema)) {
     schema->release(schema);
-    assert(ArrowSchemaIsReleased(schema));
+    ARROW_C_ASSERT(ArrowSchemaIsReleased(schema),
+                   "ArrowSchemaRelease did not cleanup release callback");
   }
 }
 
@@ -78,7 +88,8 @@ inline void ArrowArrayMove(struct ArrowArray* src, struct ArrowArray* dest) {
 inline void ArrowArrayRelease(struct ArrowArray* array) {
   if (!ArrowArrayIsReleased(array)) {
     array->release(array);
-    assert(ArrowArrayIsReleased(array));
+    ARROW_C_ASSERT(ArrowArrayIsReleased(array),
+                   "ArrowArrayRelease did not cleanup release callback");
   }
 }
 
@@ -108,7 +119,8 @@ inline void ArrowArrayStreamMove(struct ArrowArrayStream* src,
 inline void ArrowArrayStreamRelease(struct ArrowArrayStream* stream) {
   if (!ArrowArrayStreamIsReleased(stream)) {
     stream->release(stream);
-    assert(ArrowArrayStreamIsReleased(stream));
+    ARROW_C_ASSERT(ArrowArrayStreamIsReleased(stream),
+                   "ArrowArrayStreamRelease did not cleanup release callback");
   }
 }