pitrou commented on code in PR #13635:
URL: https://github.com/apache/arrow/pull/13635#discussion_r981253309
##########
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:
Should you instead test a boolean flag that was set to true in the
constructor if `UnregisterCancellingSignalHandler` was called?
--
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]
