On 06/10/11 22:36, Chris Frey wrote:
On Fri, Jun 10, 2011 at 10:37:03AM +0200, Nicolas CARRIER wrote:
Hello,
The following patch is an attempt to manage signals in pppob in a more
robust way, it uses signalfd.
* the signal interception is move into main, before the thread creation
so that signal mask is shared between threads
* a signalfd is created and appended to the fd set monitored by select.
The main advantage I see is that it allows to get rid of timeouts.
The main drawback I see is that signalfd is linux specific, hence
breaking compatibility with other unices.
Thanks! Unfortunately, this drawback is enough to kill the patch.
There are users on FreeBSD and MacOSX that need a working Barry.
Maybe this new patch, or something similar should be ok ? It uses
signalfd on Linux and replaces it by a pipe's read channel on other
platforms. Both approaches seemed to work on Linux. But I couldn't test
it on another platform as I don't have any.
What is the main problem that this patch is trying to solve? Are there
cases where the signals are not caught? I think I've seen this myself
a few times, and didn't know what was happening.
I from time to time, had pppob not terminating properly. As I couldn't
figure out why, I tried to suppress possible causes.
What is sure is that in some bad timing circumstances, the signal could
be handled right before the select statement hence resulting in a
useless 30 seconds timeout, which this kind of patch should avoid.
Perhaps the problems you experienced were due to the use of signal()
instead of sigaction() ? I know it's kind of deprecated, but I didn't
understood why.
Thanks,
- Chris
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
I had also "Exception in IpModem::DataReadThread: (-32, Pipe): Error in
BulkRead" when unplugging the blackberry in use.
The second (really simple, maybe oversimple ?) patch tries to fix this.
When an exception is catched in DataReadThread, the reading loop doesn't
terminate, resulting in a lot of error messages and CPU consumption
instead of properly quitting.
--
Nicolas CARRIER - Parrot France
--- barry-0.17.1git/tools/pppob.cc 2011-06-06 10:48:48.000000000 +0200
+++ /home/ncarrier/workspace/barry/tools/pppob.cc 2011-06-14 14:07:16.765894295 +0200
@@ -34,8 +34,26 @@
#include <unistd.h>
#include <stdlib.h>
#include <signal.h>
+#include <errno.h>
#include "i18n.h"
+#ifdef linux
+#include <sys/signalfd.h>
+#else
+/**
+ * \var __g_signalfd_emulation_pipe__
+ * \brief Global pipe used by signalfd emulation
+ */
+int __g_signalfd_emulation_pipe__[2];
+
+/**
+ * Signal handler which writes a dummy byte into the pipe simulating a signalfd
+ * @param p_signum Signal number
+ */
+void __my_signalfd_signal_handler__(int p_signum) {
+ write(__g_signalfd_emulation_pipe__[1], &p_signum, 1);
+}
+#endif
using namespace std;
using namespace Barry;
@@ -62,11 +80,6 @@
<< endl;
}
-void signal_handler(int signum)
-{
- signal_end = true;
-}
-
void SerialDataCallback(void *context, const unsigned char *data, int len)
{
if( len && data_dump )
@@ -91,41 +104,109 @@
Data data;
int bytes_read;
fd_set rfds;
- struct timeval tv;
+ int sfd;
int ret;
- // Handle interrupt signals from pppd
+ // Handle interrupt signals
+ sigset_t mask;
+
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGTERM);
+ sigaddset(&mask, SIGINT);
+ sigaddset(&mask, SIGHUP);
+
signal_end = false;
- signal(SIGINT, &signal_handler);
- signal(SIGHUP, &signal_handler);
- signal(SIGTERM, &signal_handler);
+
+#ifdef linux
+ sfd = signalfd (-1, &mask, 0);
+ if (sfd < 0) {
+ perror("signalfd");
+ return;
+ }
+#else
+ {
+ int i;
+ struct sigaction sa ;
+
+ sa.sa_handler = __my_signalfd_signal_handler__;
+ sa.sa_mask = mask;
+ sa.sa_flags = SA_RESETHAND;
+
+ if (pipe(__g_signalfd_emulation_pipe__) != 0) {
+ perror("pipe");
+ return;
+ }
+ for (i = 1; i < NSIG; i++) {
+ if (sigismember(&mask, i)) {
+ if(sigaction(i, &sa, NULL) < 0) {
+ perror("sigaction");
+ return;
+ };
+ }
+ }
+
+ sfd = __g_signalfd_emulation_pipe__[0];
+ }
+#endif
FD_ZERO(&rfds);
+
while( signal_end == false ) {
// Need to use select() here, so that pppd doesn't
// hang when it tries to set the line discipline
// on our stdin.
FD_SET(0, &rfds);
- tv.tv_sec = 30;
- tv.tv_usec = 0;
+ FD_SET(sfd, &rfds);
- ret = select(1, &rfds, NULL, NULL, &tv);
+ errno = 0;
+ ret = select(sfd + 1, &rfds, NULL, NULL, NULL);
if( ret == -1 ) {
- perror("select()");
+ if (errno != EINTR) {
+ perror("select");
+ signal_end = true;
+ }
}
else if( ret && FD_ISSET(0, &rfds) ) {
bytes_read = read(0, data.GetBuffer(), data.GetBufSize());
- if( bytes_read == 0 )
- break;
+ if( bytes_read == 0 ) break;
if( bytes_read > 0 ) {
data.ReleaseBuffer(bytes_read);
modem.Write(data);
}
}
+ else if( ret && FD_ISSET(sfd, &rfds) ) {
+ cout << "Killed by a signal, exiting" << endl;
+ signal_end = true;
+ }
+ }
+
+ close(sfd);
+}
+
+#ifdef linux
+/**
+ * Blocks signals that we handle ourselves
+ * @return non-zero on errors
+ */
+static int intercept_signals()
+{
+ sigset_t mask;
+
+ sigemptyset(&mask);
+ sigaddset(&mask, SIGTERM);
+ sigaddset(&mask, SIGINT);
+ sigaddset(&mask, SIGHUP);
+
+ if (pthread_sigmask(SIG_BLOCK, &mask, NULL) < 0) {
+ perror ("pthread_sigmask");
+ return 1;
}
+
+ return 0;
}
+#endif
int main(int argc, char *argv[])
{
@@ -134,6 +215,13 @@
cout.sync_with_stdio(true); // leave this on, since libusb uses
// stdio for debug messages
+#ifdef linux
+ if (intercept_signals() != 0) {
+ cerr << "Signal management failed" << endl;
+ return 1;
+ }
+#endif
+
try {
uint32_t pin = 0;
--- barry-0.17.1git/src/m_ipmodem.cc 2011-06-06 10:48:48.000000000 +0200
+++ /home/ncarrier/workspace/barry/src/m_ipmodem.cc 2011-06-14 15:02:31.337130983 +0200
@@ -27,6 +27,7 @@
#include "debug.h"
#include <sstream>
#include <string.h>
+#include <signal.h>
#include "sha1.h"
namespace Barry { namespace Mode {
@@ -195,6 +196,8 @@
}
catch( std::exception &e ) {
eout("Exception in IpModem::DataReadThread: " << e.what());
+ ipmodem->m_continue_reading = false;
+ kill(getpid(), SIGTERM);
}
}
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
