paleolimbot commented on code in PR #13635:
URL: https://github.com/apache/arrow/pull/13635#discussion_r982444930


##########
r/src/safe-call-into-r-impl.cpp:
##########
@@ -42,43 +57,40 @@ bool CanRunWithCapturedR() {
 std::string TestSafeCallIntoR(cpp11::function r_fun_that_returns_a_string,
                               std::string opt) {
   if (opt == "async_with_executor") {
-    std::thread* thread_ptr;
+    std::unique_ptr<std::thread> thread_ptr;

Review Comment:
   When Weston initially sketched this, he used `new std::thread()`, which is 
why it was here. He later suggested that I use `std::unique_ptr<std::thread>`, 
but this caused a crash which I didn't know how to debug at the time (this PR 
fixes that: I was throwing an exception when I should have been returning an 
error status). I think it has to be a pointer because it gets assigned as part 
of the dance of capturing the main R thread and launching another one with a 
reference to it.



-- 
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