Block SIGPIPE on background thread #169
Conversation
|
We'll test this out today. Thanks for your work on this! |
|
OK, this branch seems to fix our problems, in that the process seems unable to receive However, I did some work of my own on this yesterday and thought I'd present another approach, using libuv's signal support instead of As you can see, we are now handling the signals ourselves, and R no longer segfaults. The patch I used to achieve this is below. If you like this approach I can submit it as a PR or you can From c5c8b05e2707668cb5565ad18daac3748062ca88 Mon Sep 17 00:00:00 2001
From: Aaron Jacobs <[email protected]>
Date: Tue, 14 Aug 2018 15:36:24 +0000
Subject: [PATCH] Handle SIGPIPE via libuv. Fixes #168.
Signed-off-by: Aaron Jacobs <[email protected]>
---
src/httpuv.cpp | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/src/httpuv.cpp b/src/httpuv.cpp
index 07053d5..614b57d 100644
--- a/src/httpuv.cpp
+++ b/src/httpuv.cpp
@@ -43,6 +43,16 @@ public:
guard guard(_mutex);
if (!_initialized) {
uv_loop_init(&_loop);
+#if !defined(_WIN32)
+ // Suppress SIGPIPE.
+ int rc = uv_signal_init(&_loop, &_sigpipe_signal);
+ if (rc == 0) {
+ rc = uv_signal_start(&_sigpipe_signal, _on_sigpipe, SIGPIPE);
+ }
+ if (rc != 0) {
+ throw std::runtime_error("failed to start signal handler");
+ }
+#endif
_initialized = true;
}
}
@@ -56,6 +66,19 @@ public:
return &_loop;
};
+ int stop_signal_handler() {
+ guard guard(_mutex);
+ if (!_initialized) {
+ throw std::runtime_error("io_loop not initialized!");
+ }
+#if !defined(_WIN32)
+ // Halt SIGPIPE suppression.
+ return uv_signal_stop(&_sigpipe_signal);
+#else
return 0;
+#endif
+ }
+
void reset() {
guard guard(_mutex);
_initialized = false;
@@ -65,6 +88,13 @@ private:
uv_loop_t _loop;
uv_mutex_t _mutex;
bool _initialized;
+
+#if !defined(_WIN32)
+ uv_signal_t _sigpipe_signal;
+ static void _on_sigpipe(uv_signal_t* signal, int signum) {
+ // Do nothing, i.e. suppress SIGPIPE.
+ }
+#endif
};
// ============================================================================
@@ -116,6 +146,7 @@ void io_thread(void* data) {
trace("io_loop stopped");
// Cleanup stuff
+ io_loop.stop_signal_handler();
uv_walk(io_loop.get(), close_handle_cb, NULL);
uv_run(io_loop.get(), UV_RUN_ONCE);
uv_loop_close(io_loop.get());
--
2.7.4 |
|
@atheriel Your approach looks interesting... Are you sure that it will not affect how other threads in the process handle SIGPIPE? We would consider a PR -- @jcheng5 and I will have to figure out which approach to go with. If you want to do a PR, please fill out the contributor agreement. |
|
One more note about this PR: it makes it so the httpuv background thread stops listening for SIGPIPE. However, the process overall can still receive SIGPIPE -- it just won't be handled by the background thread. |
|
@wch After some testing, I can confirm that this PR only blocks On the other hand, my approach seems to take over signal handling for the entire process -- even if you send I'm not sure which approach is more desirable. |
|
@atheriel It sounds like libuv's @jcheng5 Do you think we need to handle/block SIGPIPE for the later package as well? |
This (hopefully) fixes #168. @shapenaji, @atheriel, can you try installing this and see if it fixes rstudio/plumber#289?
Note: The
pthread_sigmask()documentation wasn't clear about what exactly would happen if a blocked signal is sent to the thread -- would the signal be then sent to another available thread? I wrote the program below to test it out and found that no, the signal does not get sent to another thread (which is what we want).