1

Context and the problem

Recently I've been messing around with named pipes for a university project I'm working on. I need to make a program that acts as a "server" - It continuously reads from a named pipe and executes whatever command is given to it through said pipe. I've managed to do this but there's a problem: 100% CPU usage. Obviously this is a problem and it would contribute to a lower grade from my professors so I'd like to reduce this.

EDIT: I forgot to mention that after reading the message from the pipe, and executing it, the server must keep on running waiting for another message. At no point should the server program terminate (except for when SIGINT or similar signal is sent). The desired behavior is for it to never leave the loop, and just keep on reading and executing the message sent by the pipe.

Minimal reproducible example of the code for the "server":

#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <signal.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>

int main () {
    if (mkfifo("fifo", 0600) == -1) {
        perror("Error on fifo");
        return -1;
    }
    char buffer[1024];
    int bytesWritten = 0;
    int fifo = open("fifo", O_RDONLY);
    while(1) {
        bytesWritten = read(fifo, buffer, 1024);
        if (bytesWritten > 0) 
            write(1, buffer, bytesWritten);
    }
    return 0;
}

I'm not handling errors on some functions just in this particular example, on the actual project I handle all of those. This code just prints out whatever the command passed to it through the pipe.
This is a small program that I made to send something through the pipe:

#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <signal.h>

int main () {
    int fifo = open("fifo", O_WRONLY);
    write(fifo, "This is a test\n", strlen("This is a test\n"));
    close(fifo);
    return 0;
}

This works fine, apart from the CPU usage problem. One thing I've thought of was using pause() in the beginning of the while, and stopping this pause once every second.

What I tried:

#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <signal.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>

void alarm_handler(int signum) {
    alarm(1);
}

int main () {
    if (mkfifo("fifo", 0600) == -1) {
        perror("Error on fifo");
        return -1;
    }
    char buffer[1024];
    int bytesWritten = 0;
    int fifo = open("fifo", O_RDONLY);
    signal(SIGALRM, alarm_handler);
    alarm(1);
    while(1) {
        pause();
        bytesWritten = read(fifo, buffer, 1024);
        if (bytesWritten > 0) 
            write(1, buffer, bytesWritten);
    }
    return 0;
}

This "works", by setting CPU usage to 0%. However, it makes it so that my project wouldn't work properly. If I send two commands fast enough, it would register only as one command. This is due to the pause, while execution is stopped, these commands would be written on to the pipe without being read - Writing hello and world separately would register as a hello world instead of two separate arguments.
Is there a different way I can reduce CPU usage?

Ramux05
  • 332
  • 1
  • 10
  • You have to break out of the loop when `read()` returns 0 (EOF) or -1 (error). – Barmar Jun 06 '21 at 00:59
  • @Barmar Wouldn't that make it so that if nothing is being read from the other end of the pipe the program just ends? – Ramux05 Jun 06 '21 at 01:00
  • 1
    I would say one way is to use something like `poll` to check whether the descriptor is ready for reading and only then read from it. But honestly with a single descriptor to read from, the blocking `read` call should not eat up CPU by itself. You do need to handle any read errors though, once it starts failing, the fifo will likely not recover during the lifetime of your program. – Cheatah Jun 06 '21 at 01:19
  • As a comment to the secondary "problem" you describe: if you need to distinguish between "messages" or "commands", make sure you use a protocol that is capable of doing so. Even a simple length-value protocol should suffice. – Cheatah Jun 06 '21 at 01:20
  • @Cheatah I thought that because the pipe would be blocked (from what I understand, it the other end is closed the read would block), but still... 100% cpu usage I don't know why... I will look into poll() though, after a quick read about it it looks like it might be the solution for my problem! Thank you :) – Ramux05 Jun 06 '21 at 01:24
  • @Cheatah Unfortunately I can't do it by length as It may vary.. The poll() solution you suggested might do the trick and avoid the problem altogether though, thank you – Ramux05 Jun 06 '21 at 01:25
  • @Ramux05 By default `read` is blocking until some data is received, so no, the programme wouldn't just end... – Aconcagua Jun 06 '21 at 01:26
  • @Aconcagua True, but I do need to keep on reading after executing one command, breaking out of the loop would make it so that the program would end after only one command, or am I misunderstanding? – Ramux05 Jun 06 '21 at 01:27
  • [This post](https://stackoverflow.com/questions/14594508/fifo-pipe-is-always-readable-in-select) (and especially the accepted answer) might help. – Paul Sanders Jun 06 '21 at 01:32
  • Read blocks until data is available, and you can repeat that again and again. Read will return 0 on pipe being closed or -1 on error. In the latter case you might check `errno` for being `EINTR` – incoming signals would break reading as well, in that case you could go on reading. – Aconcagua Jun 06 '21 at 01:36
  • By the way, in general it is not a good idea to rely on time to separate incoming messages, even though in some specific cases that might suffice. Safer in any case is using a separator character. If your data can contain arbitrary characters [COBS](https://en.wikipedia.org/wiki/Consistent_Overhead_Byte_Stuffing) might be interesting for you. – Aconcagua Jun 06 '21 at 01:39
  • Please put in the error handling, because that's where the bug is. – Nate Eldredge Jun 06 '21 at 01:52
  • @EricPostpischil tried your suggestion and it didn't work. It's my fault for not explaining properly: After I'm done executing some command given to me by the user, the server is supposed to keep on waiting for more input, not terminate after it's done executing it – Ramux05 Jun 06 '21 at 01:58
  • @EricPostpischil the point is that I can (for example) keep on executing the client executable and keep printing (in this case) the same message. On the actual project the client is called with arguments, and sends a message to the server depending on those arguments: ```./client status``` would be asking for the server's status, for instance – Ramux05 Jun 06 '21 at 02:03
  • @Aconcagua I will look into assigning separator characters, thank you. – Ramux05 Jun 06 '21 at 02:16

2 Answers2

1

@Cheatah's sugestion of using poll() worked perfectly. This is the code now with the changes suggested:

#include <unistd.h>
#include <fcntl.h>
#include <string.h>
#include <signal.h>
#include <stdio.h>
#include <sys/types.h>
#include <stdlib.h>
#include <sys/stat.h>
#include <poll.h>

int main () {
    if (mkfifo("fifo", 0600) == -1) {
        perror("Error on fifo");
        return -1;
    }

    char buffer[1024];
    int bytesWritten = 0;
    int fifo = open("fifo", O_RDONLY);
    struct pollfd *pfd = calloc(1, sizeof(struct pollfd));
    pfd->fd = fifo;
    pfd->events = POLLIN;
    pfd->revents = POLLOUT;

    while(1) {
        poll(pfd, 1, -1);
        bytesWritten = read(fifo, buffer, 1024);
        if (bytesWritten == -1) {
            perror("Error on read()");
            exit(-1);
        }
        if (bytesWritten > 0) 
            write(1, buffer, bytesWritten);
    }
    return 0;
}

This gets me 0% CPU usage and works really well

Ramux05
  • 332
  • 1
  • 10
1

A minimal change in your code to prevent this busy-waiting condition is to simply open the fifo in your server as read/write instead of read-only:

int fifo = open("fifo", O_RDWR);

By this way, you will always have the fifo opened for writing by a program (the server itself) and so the read operation will not have this premature termination when the first client closes the fifo.

Using poll() on the other hand is specially good if you want your read() to block with a timeout.

See: https://stackoverflow.com/a/23499902/6874310

Jardel Lucca
  • 1,115
  • 3
  • 10
  • 19