paleolimbot commented on code in PR #13635:
URL: https://github.com/apache/arrow/pull/13635#discussion_r982461739
##########
r/src/safe-call-into-r.h:
##########
@@ -75,61 +120,113 @@ class MainRThread {
// Check if there is a saved error
bool HasError() { return !status_.ok(); }
- // Throw a cpp11::unwind_exception() if
- void ClearError() {
+ // Resets this object after a RunWithCapturedR is about to return
+ // to the R interpreter.
+ arrow::Status ReraiseErrorIfExists() {
+ if (SignalStopSourceEnabled()) {
+ stop_source_->Reset();
+ }
arrow::Status maybe_error_status = status_;
ResetError();
- arrow::StopIfNotOk(maybe_error_status);
+ return maybe_error_status;
}
private:
bool initialized_;
std::thread::id thread_id_;
arrow::Status status_;
arrow::internal::Executor* executor_;
+ arrow::StopSource* stop_source_;
+
+ MainRThread() : initialized_(false), executor_(nullptr),
stop_source_(nullptr) {}
};
-// Retrieve the MainRThread singleton
-MainRThread& GetMainRThread();
+// This object is used to ensure that signal hanlders are registered when
+// RunWithCapturedR launches its background thread to call Arrow and is
+// cleaned up however this exits. Note that the lifecycle of the StopSource,
+// which is registered at package load, is not necessarily tied to the
+// lifecycle of the signal handlers. The general approach is to register
+// the signal handlers only when we are evaluating code outside the R thread
+// (when we are evaluating code *on* the R thread, R's signal handlers are
+// sufficient and will signal an interupt condition that will propagate
+// via a cpp11::unwind_excpetion).
+class RunWithCapturedRContext {
+ public:
+ arrow::Status Init() {
+ if (MainRThread::GetInstance().SignalStopSourceEnabled()) {
+ RETURN_NOT_OK(arrow::RegisterCancellingSignalHandler({SIGINT}));
+ }
+
+ return arrow::Status::OK();
+ }
+
+ ~RunWithCapturedRContext() {
+ if (MainRThread::GetInstance().SignalStopSourceEnabled()) {
Review Comment:
Done!
##########
r/src/safe-call-into-r.h:
##########
@@ -75,61 +120,113 @@ class MainRThread {
// Check if there is a saved error
bool HasError() { return !status_.ok(); }
- // Throw a cpp11::unwind_exception() if
- void ClearError() {
+ // Resets this object after a RunWithCapturedR is about to return
+ // to the R interpreter.
+ arrow::Status ReraiseErrorIfExists() {
+ if (SignalStopSourceEnabled()) {
+ stop_source_->Reset();
+ }
arrow::Status maybe_error_status = status_;
ResetError();
- arrow::StopIfNotOk(maybe_error_status);
+ return maybe_error_status;
}
private:
bool initialized_;
std::thread::id thread_id_;
arrow::Status status_;
arrow::internal::Executor* executor_;
+ arrow::StopSource* stop_source_;
+
+ MainRThread() : initialized_(false), executor_(nullptr),
stop_source_(nullptr) {}
};
-// Retrieve the MainRThread singleton
-MainRThread& GetMainRThread();
+// This object is used to ensure that signal hanlders are registered when
+// RunWithCapturedR launches its background thread to call Arrow and is
+// cleaned up however this exits. Note that the lifecycle of the StopSource,
+// which is registered at package load, is not necessarily tied to the
+// lifecycle of the signal handlers. The general approach is to register
+// the signal handlers only when we are evaluating code outside the R thread
+// (when we are evaluating code *on* the R thread, R's signal handlers are
+// sufficient and will signal an interupt condition that will propagate
+// via a cpp11::unwind_excpetion).
+class RunWithCapturedRContext {
+ public:
+ arrow::Status Init() {
+ if (MainRThread::GetInstance().SignalStopSourceEnabled()) {
+ RETURN_NOT_OK(arrow::RegisterCancellingSignalHandler({SIGINT}));
+ }
+
+ return arrow::Status::OK();
+ }
+
+ ~RunWithCapturedRContext() {
+ if (MainRThread::GetInstance().SignalStopSourceEnabled()) {
+ arrow::UnregisterCancellingSignalHandler();
+ }
+ }
+};
+
+// This is an object whose scope ensures we do not register signal handlers
when
+// evaluating R code when that evaluation happens via SafeCallIntoR.
+class SafeCallIntoRContext {
+ public:
+ SafeCallIntoRContext() {
+ if (!MainRThread::GetInstance().IsMainThread() &&
+ MainRThread::GetInstance().SignalStopSourceEnabled()) {
+ arrow::UnregisterCancellingSignalHandler();
+ }
+ }
+
+ ~SafeCallIntoRContext() {
+ if (!MainRThread::GetInstance().IsMainThread() &&
+ MainRThread::GetInstance().SignalStopSourceEnabled()) {
Review Comment:
Done!
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
