taepper commented on code in PR #51038:
URL: https://github.com/apache/arrow/pull/51038#discussion_r4194443496


##########
cpp/src/arrow/json/parser.cc:
##########
@@ -662,71 +657,58 @@ class HandlerBase : public BlockParser,
     return builder_set_.Cast<kind>(builder);
   }
 
-  /// Accessor for a stored error Status
-  Status Error() { return status_; }
-
-  /// \defgroup rapidjson-handler-interface functions expected by rj::Reader
-  ///
-  /// bool Key(const char* data, rj::SizeType size, ...) is omitted since
-  /// the behavior varies greatly between UnexpectedFieldBehaviors
-  ///
-  /// @{
-  bool Null() {
-    status_ = builder_set_.AppendNull(builder_stack_.back(), field_index_, 
builder_);
-    return status_.ok();
+  Status Null() {
+    return builder_set_.AppendNull(builder_stack_.back(), field_index_, 
builder_);

Review Comment:
   ```
   python -c "
   import io, pyarrow.json as j
   j.read_json(io.BytesIO(b'null\n'))"
   ```
   
   oh, sorry for the noise. I accidentally reused a stale RapidJSON version of 
the parser that I still had locally. After correctly rebuilding this branch's 
version it does not segfault anymore. So this actually fixed a the segfault in 
the prior code
   
   
   <details>
   <summary>this is the outdated stacktrace for arrow 25.0.1</summary>
   
   ```
     Thread 18 "python" received signal SIGSEGV, Segmentation fault.
     [Switching to Thread 0x7fffc56f06c0 (LWP 2878766)]
     0x00007ffff492e166 in arrow::json::HandlerBase::Null (this=0x7fffc0000f10) 
at /home/alexander/arrow/cpp/src/arrow/json/parser.cc:675
     #0  0x00007ffff492e166 in arrow::json::HandlerBase::Null 
(this=0x7fffc0000f10) at /home/alexander/arrow/cpp/src/arrow/json/parser.cc:675
     #1  0x00007ffff494504a in 
arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, arrow::rapidjson::CrtAllocator>::ParseNull<332u, 
     arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::MemoryStream>, 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> > 
(this=0x7fffc56ef890, is=..., handler=...) at 
     /usr/include/rapidjson/reader.h:710
     #2  0x00007ffff493de07 in 
arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, arrow::rapidjson::CrtAllocator>::ParseValue<332u, 
     arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::MemoryStream>, 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> > 
(this=0x7fffc56ef890, is=..., handler=...) at 
     /usr/include/rapidjson/reader.h:1394
     #3  0x00007ffff4944f47 in 
arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, arrow::rapidjson::CrtAllocator>::Transit<332u, 
     arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::MemoryStream>, 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> > 
(this=0x7fffc56ef890, 
     src=arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::CrtAllocator>::IterativeParsingStartState, 
     token=arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, arrow::rapidjson::CrtAllocator>::NullToken, 
dst=arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
     arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::CrtAllocator>::IterativeParsingValueState, is=..., 
handler=...) at /usr/include/rapidjson/reader.h:1792
     #4  0x00007ffff493dc5f in 
arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::CrtAllocator>::IterativeParse<332u, 
     arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::MemoryStream>, 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> > 
(this=0x7fffc56ef890, is=..., handler=...) at 
     /usr/include/rapidjson/reader.h:1832
     #5  0x00007ffff493a9e9 in 
arrow::rapidjson::GenericReader<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::UTF8<char>, arrow::rapidjson::CrtAllocator>::Parse<332u, 
     arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
arrow::rapidjson::MemoryStream>, 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2> > 
(this=0x7fffc56ef890, is=..., handler=...) at 
     /usr/include/rapidjson/reader.h:487
     #6  0x00007ffff49369d5 in 
arrow::json::HandlerBase::DoParse<arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>,
 arrow::rapidjson::EncodedInputStream<arrow::rapidjson::UTF8<char>, 
     arrow::rapidjson::MemoryStream> > (this=0x7fffc0000f10, handler=..., 
json=..., json_size=5) at /home/alexander/arrow/cpp/src/arrow/json/parser.cc:774
     #7  0x00007ffff4932f80 in 
arrow::json::HandlerBase::DoParse<arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>
 > (this=0x7fffc0000f10, handler=..., json=std::shared_ptr<arrow::Buffer> (use 
count 2, weak 
     count 0) = {...}) at /home/alexander/arrow/cpp/src/arrow/json/parser.cc:801
     #8  0x00007ffff492fe9d in 
arrow::json::Handler<(arrow::json::UnexpectedFieldBehavior)2>::Parse 
(this=0x7fffc0000f10, json=std::shared_ptr<arrow::Buffer> (use count 2, weak 
count 0) = {...}) at 
     /home/alexander/arrow/cpp/src/arrow/json/parser.cc:1081
     #9  0x00007ffff494b1c4 in arrow::json::(anonymous namespace)::ParseBlock 
(block=..., parse_options=..., pool=0x7ffff691c300 <arrow::global_state+64>, 
out_size=0x0) at 
     /home/alexander/arrow/cpp/src/arrow/json/reader.cc:162
     #10 0x00007ffff494ce78 in arrow::json::(anonymous 
namespace)::TableReaderImpl::ParseAndInsert (this=0xff8d50, block=...) at 
/home/alexander/arrow/cpp/src/arrow/json/reader.cc:287
     #11 0x00007ffff494c5d7 in operator() (__closure=0xfeb788) at 
/home/alexander/arrow/cpp/src/arrow/json/reader.cc:268
     #12 0x00007ffff4968955 in 
arrow::internal::FnOnce<arrow::Status()>::FnImpl<arrow::json::(anonymous 
namespace)::TableReaderImpl::Read()::<lambda()> >::invoke(void) (this=0xfeb780) 
at 
     /home/alexander/arrow/cpp/src/arrow/util/functional.h:96
     #13 0x00007ffff49c6cdc in arrow::internal::FnOnce<arrow::Status 
()>::operator()() && (this=0xf7f0e8) at 
/home/alexander/arrow/cpp/src/arrow/util/functional.h:84
     #14 0x00007ffff4cd0765 in operator() (__closure=0xf7f0d8) at 
/home/alexander/arrow/cpp/src/arrow/util/task_group.cc:114
     #15 0x00007ffff4cd218c in 
arrow::internal::FnOnce<void()>::FnImpl<arrow::internal::(anonymous 
namespace)::ThreadedTaskGroup::AppendReal(arrow::internal::FnOnce<arrow::Status()>)::<lambda()>
 >::invoke(void) 
     (this=0xf7f0d0) at /home/alexander/arrow/cpp/src/arrow/util/functional.h:96
     #16 0x00007ffff4ce30db in arrow::internal::FnOnce<void ()>::operator()() 
&& (this=0x7fffc56efd50) at 
/home/alexander/arrow/cpp/src/arrow/util/functional.h:84
     #17 0x00007ffff4cdd012 in arrow::internal::WorkerLoop 
(state=std::shared_ptr<arrow::internal::ThreadPool::State> (use count 3, weak 
count 1) = {...}, it=...) at 
     /home/alexander/arrow/cpp/src/arrow/util/thread_pool.cc:482
     #18 0x00007ffff4cde13b in operator() (__closure=0xceb588) at 
/home/alexander/arrow/cpp/src/arrow/util/thread_pool.cc:705
     #19 0x00007ffff4ce28b6 in std::__invoke_impl<void, 
arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::<lambda()> 
>(std::__invoke_other, struct {...} &&) (__f=...) at 
/usr/include/c++/13/bits/invoke.h:61
     #20 0x00007ffff4ce2879 in 
std::__invoke<arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::<lambda()>
 >(struct {...} &&) (__fn=...) at /usr/include/c++/13/bits/invoke.h:96
     #21 0x00007ffff4ce2826 in 
std::thread::_Invoker<std::tuple<arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::<lambda()>
 > >::_M_invoke<0>(std::_Index_tuple<0>) (this=0xceb588) at 
     /usr/include/c++/13/bits/std_thread.h:292
     #22 0x00007ffff4ce27fa in 
std::thread::_Invoker<std::tuple<arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::<lambda()>
 > >::operator()(void) (this=0xceb588) at 
/usr/include/c++/13/bits/std_thread.h:299
     #23 0x00007ffff4ce27de in 
std::thread::_State_impl<std::thread::_Invoker<std::tuple<arrow::internal::ThreadPool::LaunchWorkersUnlocked(int)::<lambda()>
 > > >::_M_run(void) (this=0xceb580) at 
     /usr/include/c++/13/bits/std_thread.h:244
     #24 0x00007ffff20ecdb4 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
     #25 0x00007ffff7c9caa4 in start_thread (arg=<optimized out>) at 
./nptl/pthread_create.c:447
     #26 0x00007ffff7d29c6c in clone3 () at 
../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
   ```
   
   </details>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to