1

This post is related to: POSIX TIMER- Have multiple timers

I want to call a function in the SignalHandler. This function is a TCP socket client(getSpeed). It gets data from a server every time the timer ticks one second and sends a signal which then calls the corresponding handler.

First of all I am not sure if it is good practise to call a function from a signal handler.

Now the problem is, my code freezes randomly whenever I execute this program which talks to a server.

If this way of defining a timer using signals is bad(and probably causes some hidden race condition) is there a better way to code the above scenario?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <signal.h>
#include <netinet/in.h>
#include <linux/socket.h>
#include <time.h>


static timer_t     tid;
static timer_t     tid2;

void SignalHandler(int, siginfo_t*, void* );
timer_t SetTimer(int, int, int);

int getSpeed(void) {

    int sockFD = 0;
    struct sockaddr_in addr;
    int numbytes;
    unsigned char rxMsg[128];

    /* open socket */
    if((sockFD = socket(AF_INET, SOCK_STREAM, 0)) <= 0) {
        fprintf(stderr, "%s: error while creating socket\n", __func__);
        return -1;
    }

    memset(&addr, 0, sizeof(addr));
    addr.sin_family = AF_INET;
    inet_aton(IP, (struct in_addr *)(&(addr.sin_addr)));
    addr.sin_port = htons(DATA_PORT);
    if (connect(sockFD,(struct sockaddr *) &addr,sizeof(addr)) < 0)
        fprintf(stderr, "%s: failed to connect to server\n", __func__);

    if((numbytes = recv(sockFD, rxMsg, sizeof(rxMsg), 0)) < 1){
        fprintf(stderr, "%s: failed to recv data from vehicle\n", __func__);
        close(sockFD);
        return -1;
    }
    printf("bytes received from Client is : %d\n", numbytes);

    printf("Data  = %s\n",rxMsg);

    // close socket
    close(sockFD);
    return 0;
}
int main(int argc, char *argv[]) {


    struct sigaction sigact;
    sigemptyset(&sigact.sa_mask);
    sigact.sa_flags = SA_SIGINFO;
    sigact.sa_sigaction = SignalHandler;
    // set up sigaction to catch signal
    if (sigaction(SIGTIMER, &sigact, NULL) == -1) {
        perror("sigaction failed");
        exit( EXIT_FAILURE );
    }

    // Establish a handler to catch CTRL+c and use it for exiting.
    sigaction(SIGINT, &sigact, NULL);
    tid=SetTimer(SIGTIMER, 1000, 1);

    struct sigaction sa;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_SIGINFO;
    sa.sa_sigaction = SignalHandler;
    // set up sigaction to catch signal
    if (sigaction(SIG, &sa, NULL) == -1) {
        perror("sa failed");
        exit( EXIT_FAILURE );
    }

    // Establish a handler to catch CTRL+c and use it for exiting.
    sigaction(SIGINT, &sa, NULL);
    tid2=SetTimer(SIG, 1000, 1);
    for(;;);
    return 0;
}

void SignalHandler(int signo, siginfo_t* info, void* context)
{
    if (signo == SIGTIMER) {
        //printf("Command Caller has ticked\n");

    }else if (signo == SIG) {
        //printf("Data Caller has ticked\n");
        getData();

    } else if (signo == SIGINT) {
        timer_delete(tid);
        timer_delete(tid2);
        perror("Crtl+c cached!");
        exit(1);  // exit if CRTL/C is issued
    }
}
timer_t SetTimer(int signo, int sec, int mode)
{
    static struct sigevent sigev;
    static timer_t tid;
    static struct itimerspec itval;
    static struct itimerspec oitval;

    // Create the POSIX timer to generate signo
    sigev.sigev_notify = SIGEV_SIGNAL;
    sigev.sigev_signo = signo;
    sigev.sigev_value.sival_ptr = &tid;

    if (timer_create(CLOCK_REALTIME, &sigev, &tid) == 0) {
        itval.it_value.tv_sec = sec / 1000;
        itval.it_value.tv_nsec = (long)(sec % 1000) * (1000000L);

        if (mode == 1) {
            itval.it_interval.tv_sec = itval.it_value.tv_sec;
            itval.it_interval.tv_nsec = itval.it_value.tv_nsec;
        }
        else {
            itval.it_interval.tv_sec = 0;
            itval.it_interval.tv_nsec = 0;
        }

        if (timer_settime(tid, 0, &itval, &oitval) != 0) {
            perror("time_settime error!");
        }
    }
    else {
        perror("timer_create error!");
        return NULL;
    }
    return tid;
}
Community
  • 1
  • 1
user489152
  • 907
  • 1
  • 23
  • 42
  • are you sure this is the best approach? why not have a thread (or your main thread) that measures elapsed time with `gettimeofday()` and then that launch the task? That would not be TO THE MILLISECOND accurate, but would be within +- 10ms in most Linuxes. – Vinicius Kamakura Jun 22 '11 at 13:52
  • I added a pthread condition variable which is set when the timer expires. Now the getspeed() (changed to a thread) waits on this variable to do socket communication. It works fine, but I'd like to hear all your opinions on that – user489152 Jun 28 '11 at 09:25

1 Answers1

2

The POSIX spec tells you exactly what API functions you may safely call from a signal handler (there is a list near the end of section 2.4.3).

socket, connect, recv, and close are all on the list, so they should be safe. printf and fprintf are not, which is not surprising since they manage a user-space buffer, which is hard to keep consistent in the presence of async signals.

So other than the printf type calls, this code actually looks OK to me. When your "code freezes", have you tried attaching with a debugger to see where you are stuck?

That said, the usual way to handle this is one of two ways.

First way: If your application has an "event loop", arrange for the signal simply to send an event. Then your main thread handles that event just like any other, and everything stays nice and synchronous.

Second way: Devote a thread to the periodic task. The thread can just loop calling sleep(1) and performing your periodic task.

But I still think your approach here should be working.

Nemo
  • 70,042
  • 10
  • 116
  • 153
  • Thanks Nemo :) The first thing I tried now was to comment out printf and tested about 12-13 times and till now this freezing didnt pop up. (As a second measure, I also tried to put two signal handler routines to separate SIG and SIGTIMER and also commented printf here).I like the thread idea for the periodic task. but dont want to have sleep called in it. Can I implement it any other way using a spearate thread? As in call a timer for that thread or something? – user489152 Jun 22 '11 at 14:02
  • Why do you not want to call sleep() in the thread? If you are worried that sleep() itself might set a timer, you can just use select() (on zero descriptors) with a timeout. This should be very efficient; select() will block, the kernel will put the thread to sleep and it will consume zero resources until the kernel decides to wake it up... – Nemo Jun 22 '11 at 14:21
  • oh ok. I will try that. Actually now my code again freezes(after removing the printfs :(...) what should I do? – user489152 Jun 22 '11 at 14:28
  • Attach with gdb and look at the backtrace. – Nemo Jun 22 '11 at 14:31
  • u mean a socket select with sleep? – user489152 Jun 22 '11 at 15:00
  • 1
    [select](http://pubs.opengroup.org/onlinepubs/9699919799/functions/select.html) takes three sets of file descriptors and a timeout. If the sets are empty, you can use the timeout alone to perform a high-resolution "sleep". Or if you only care about modern POSIX systems, just use [nanosleep](http://pubs.opengroup.org/onlinepubs/9699919799/functions/nanosleep.html), which is guaranteed to have "no effect on the action or blockage of any signal." – Nemo Jun 22 '11 at 15:04
  • `inet_aton` is also unsafe in a signal handler, note – bdonlan Jun 22 '11 at 18:20