Sockets using poll()

Hi,
Until recently I have been using select() to wait for activity on file descriptors, however this seems less efficient than poll. I rewrote the program using poll:

Server:

Code:

---

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


#define POLL_SIZE 32
#define LISTEN_QUEUE 5
#define SOCKET_PATH "web/server_socket.sock"


int main()
{
    int server_sockfd, client_sockfd;
    int server_len, client_len;
    struct sockaddr_un server_address;
    struct sockaddr_un client_address;
    int result;
   
    struct pollfd poll_set[POLL_SIZE];
    int numfds = 0;
    int max_fd = 0;
   
    unlink(SOCKET_PATH);
    server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
   
    server_address.sun_family = AF_UNIX;
    strcpy(server_address.sun_path, SOCKET_PATH);
    server_len = sizeof(server_address);
   
    bind(server_sockfd, (struct sockaddr *)&server_address, (socklen_t)server_len);
    listen(server_sockfd, 5);
   
    poll_set[0].fd = server_sockfd;
    poll_set[0].events = POLLIN;
    numfds++;
    //signal(SIGPIPE, SIG_IGN);
    update_maxfd(server_sockfd, &max_fd);


    while (1) {
        char ch;
        int fd_index;
        int nread;
       
        printf("Waiting for client (%d total)...\n", numfds);
        //result = select( (max_fd + 1), &testfds, (fd_set *)0, (fd_set *)0, (struct timeval *) 0);
        poll(poll_set, numfds, 10000);
        for(fd_index = 0; fd_index < numfds; fd_index++)
        {
            if( poll_set[fd_index].revents & POLLIN ) {
                if(poll_set[fd_index].fd == server_sockfd) {
                    client_len = sizeof(client_address);
                    client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, (socklen_t *)&client_len);
                   
                    poll_set[numfds].fd = client_sockfd;
                    poll_set[numfds].events = POLLIN;
                    numfds++;
                    update_maxfd(client_sockfd, &max_fd);


                    printf("Adding client on fd %d\n", client_sockfd);
                }
                else {
                    ioctl(poll_set[fd_index].fd, FIONREAD, &nread);
                   
                    if( nread == 0 )
                    {
                        close(poll_set[fd_index].fd);
                       
                        numfds--;
                       
                       
                        poll_set[fd_index].events = 0;
                        printf("Removing client on fd %d\n", poll_set[fd_index].fd);
                        poll_set[fd_index].fd = -1;
                    }
               
                else {
                    read(poll_set[fd_index].fd, &ch, 1);
                    printf("Serving client on fd %d\n", poll_set[fd_index].fd);
                    ch++;
                    write(poll_set[fd_index].fd, &ch, 1);
                }
            }
        }
    }
       
    }
}

---

However, whilst the first to clients connected work ok, the others get 'broken pipe' and I am unsure how to fix this.

Client:

Code:

---


#include <sys/types.h>
#include <sys/socket.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/un.h>
#include <unistd.h>


#define SOCKET_PATH "web/server_socket.sock"


int main()
{
    int sockfd;
    int len;
    struct sockaddr_un address;
    int result;
    char ch = 'A';


    /* -AF_UNIX is a UNIX internal socket, created on the filesystem.
    * -SOCK_STREAM is a reliable, sequenced, connection-based two-way byte stream
    *  (INET equivalent is TCP) */
   
    sockfd = socket(AF_UNIX, SOCK_STREAM, 0);


    /* -sun_family - specifies type (domain) of socket
    * -sun_path - socket path location (on filesystem) */


    address.sun_family = AF_UNIX;
    strcpy(address.sun_path, SOCKET_PATH);
    len = sizeof(address);


    /* request connection from the server, connecting socket to server with specified address
    * 'len' is the length of the address */
    result = connect(sockfd, (struct sockaddr *)&address, len);


    /* exit with error if connection is not successful */
    if(result == -1) {
        perror("oops: client1");
        exit(1);
    }


    /* otherwise write data to server and obtain response */
    write(sockfd, &ch, 1);
    read(sockfd, &ch, 1);
    printf("char from server: %c\n", ch);
    close(sockfd);


    exit(0);
}

---

Thanks in advance
Joshun

Broken pipes are usually reported when the peer (the process at the other end of the pipe) has closed it. You might want to check your indenting - it's probable that your code isn't grouped quite as you think .....

Not related to your question, by what measure is your code polling code more efficient than using select? Roughly speaking, the advantage of using select() is often runtime efficiency (e.g. less need to consume CPU cycles looping to poll).

also I would suggest to review what will happen with socket at index 1 after the socket at index 0 is closed (when you have 2 open sockets)

I've checked the indentation, and whilst it may look slightly confusing the braces appear to be where they should be. The server's socket for incoming connections is always open, so vart what you mentioned shouldn't be an issue. The problem appears to be with reducing 'numfds' by 1 each time a socket fd is removed - commenting this out stops the but means that pollfd structure elements are not reused (so could run out). Is there any way around this, I can' think of a way of safely reducing the number of fds. The client code should be fine as this works with an earlier server using select().

Also grumpy, when I use select I keep track of the highest numbered fd open and pass that instead of FD_SETSIZE. Whilst efficiency shouldn't be too bad I've been told its better to use poll().

there is a way - instead of setting fd to -1 - copy the last fd into the freed slot.

Quote:

---

Originally Posted by vart

there is a way - instead of setting fd to -1 - copy the last fd into the freed slot.

---

I've experimented a bit and tried what you have suggest, I also realised that I forgot to initialise the struct.

It seems to work :redface: I just hope there isn't some strange error still lurking around...

server code:

Code:

---

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


#define POLL_SIZE 32
#define LISTEN_QUEUE 5
#define SOCKET_PATH "web/server_socket.sock"


#define TRUE 1
#define FALSE 0




int main()
{
        int server_sockfd, client_sockfd;
        int server_len, client_len;
        struct sockaddr_un server_address;
        struct sockaddr_un client_address;
        int result;
       
        struct pollfd poll_set[POLL_SIZE];
        int numfds = 0;
        int max_fd = 0;
       
       
        unlink(SOCKET_PATH);
        server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
       
        server_address.sun_family = AF_UNIX;
        strcpy(server_address.sun_path, SOCKET_PATH);
        server_len = sizeof(server_address);
       
        bind(server_sockfd, (struct sockaddr *)&server_address, (socklen_t)server_len);
        listen(server_sockfd, 5);
        memset(poll_set, '\0', sizeof(poll_set));
        poll_set[0].fd = server_sockfd;
        poll_set[0].events = POLLIN;
        numfds++;
        //signal(SIGPIPE, SIG_IGN);
       
        max_fd = server_sockfd;
       
        while (1) {
                char ch;
                int fd_index;
                int nread;
               
                printf("Waiting for client (%d total)...\n", numfds);
                //result = select( (max_fd + 1), &testfds, (fd_set *)0, (fd_set *)0, (struct timeval *) 0);
                poll(poll_set, numfds, -1);
                for(fd_index = 0; fd_index < numfds; fd_index++)
                {
                        if( poll_set[fd_index].revents & POLLIN ) {
                                if(poll_set[fd_index].fd == server_sockfd) {
                                        client_len = sizeof(client_address);
                                        client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, (socklen_t *)&client_len);
                                       
                                        poll_set[numfds].fd = client_sockfd;
                                        poll_set[numfds].events = POLLIN;
                                        numfds++;


                                        printf("Adding client on fd %d\n", client_sockfd);
                                }
                                else {
                                        ioctl(poll_set[fd_index].fd, FIONREAD, &nread);
                                       
                                        if( nread == 0 )
                                        {
                                                close(poll_set[fd_index].fd);                       
                                                poll_set[fd_index].events = 0;
                                                printf("Removing client on fd %d\n", poll_set[fd_index].fd);
                                                int i;
                                                for( i = fd_index; i<numfds; i++)
                                                {
                                                        poll_set[i] = poll_set[i + 1];
                                                }
                                                numfds--;
                                        }
                               
                                else {
                                        read(poll_set[fd_index].fd, &ch, 1);
                                        printf("Serving client on fd %d\n", poll_set[fd_index].fd);
                                        ch++;
                                        write(poll_set[fd_index].fd, &ch, 1);
                                }
                        }
                }
                       
                       
          }
               
               
               
    }
}

---

1. you could use something like astyle to align you code automatically if you are too lazy to make it manually

Code:

---

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


#define POLL_SIZE 32
#define LISTEN_QUEUE 5
#define SOCKET_PATH "web/server_socket.sock"


#define TRUE 1
#define FALSE 0




int main() {
    int server_sockfd, client_sockfd;
    int server_len, client_len;
    struct sockaddr_un server_address;
    struct sockaddr_un client_address;
    int result;

    struct pollfd poll_set[POLL_SIZE];
    int numfds = 0;
    int max_fd = 0;


    unlink(SOCKET_PATH);
    server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0);

    server_address.sun_family = AF_UNIX;
    strcpy(server_address.sun_path, SOCKET_PATH);
    server_len = sizeof(server_address);

    bind(server_sockfd, (struct sockaddr *)&server_address, (socklen_t)server_len);
    listen(server_sockfd, 5);
    memset(poll_set, '\0', sizeof(poll_set));
    poll_set[0].fd = server_sockfd;
    poll_set[0].events = POLLIN;
    numfds++;
    //signal(SIGPIPE, SIG_IGN);

    max_fd = server_sockfd;

    while (1) {
        char ch;
        int fd_index;
        int nread;

        printf("Waiting for client (%d total)...\n", numfds);
        //result = select( (max_fd + 1), &testfds, (fd_set *)0, (fd_set *)0, (struct timeval *) 0);
        poll(poll_set, numfds, -1);
        for(fd_index = 0; fd_index < numfds; fd_index++) {
            if( poll_set[fd_index].revents & POLLIN ) {
                if(poll_set[fd_index].fd == server_sockfd) {
                    client_len = sizeof(client_address);
                    client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, (socklen_t *)&client_len);

                    poll_set[numfds].fd = client_sockfd;
                    poll_set[numfds].events = POLLIN;
                    numfds++;


                    printf("Adding client on fd %d\n", client_sockfd);
                } else {
                    ioctl(poll_set[fd_index].fd, FIONREAD, &nread);

                    if( nread == 0 ) {
                        close(poll_set[fd_index].fd);
                        poll_set[fd_index].events = 0;
                        printf("Removing client on fd %d\n", poll_set[fd_index].fd);
                        int i;
                        for( i = fd_index; i<numfds; i++) {
                            poll_set[i] = poll_set[i + 1];
                        }
                        numfds--;
                    }

                    else {
                        read(poll_set[fd_index].fd, &ch, 1);
                        printf("Serving client on fd %d\n", poll_set[fd_index].fd);
                        ch++;
                        write(poll_set[fd_index].fd, &ch, 1);
                    }
                }
            }


        }



    }
}

---

2. gcc gives warnings about initialized but not used variable maxfd
3. You do not need to move all fds - just last one into the free slot (lines 88/90)

Thanks for the advice, I'll take out maxfd since it is no longer needed, and will sort out indentation with indent or astyle.

I've changed the code so it only moves the last one into the free slot, seems to work:

Code:

---

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


#define POLL_SIZE 32
#define LISTEN_QUEUE 5
#define SOCKET_PATH "web/server_socket.sock"


#define TRUE 1
#define FALSE 0




int main()
{
    int server_sockfd, client_sockfd;
    int server_len, client_len;
    struct sockaddr_un server_address;
    struct sockaddr_un client_address;
    int result;


    struct pollfd poll_set[POLL_SIZE];
    int numfds = 0;


    unlink(SOCKET_PATH);
    server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0);


    server_address.sun_family = AF_UNIX;
    strcpy(server_address.sun_path, SOCKET_PATH);
    server_len = sizeof(server_address);


    bind(server_sockfd, (struct sockaddr *)&server_address, (socklen_t)server_len);
    listen(server_sockfd, 5);
    memset(poll_set, '\0', sizeof(poll_set));
    poll_set[0].fd = server_sockfd;
    poll_set[0].events = POLLIN;
    numfds++;
    //signal(SIGPIPE, SIG_IGN);






    while (1) {
        char ch;
        int fd_index;
        int nread;


        printf("Waiting for client (%d total)...\n", numfds);
        //result = select( (max_fd + 1), &testfds, (fd_set *)0, (fd_set *)0, (struct timeval *) 0);
        poll(poll_set, numfds, -1);
        for(fd_index = 0; fd_index < numfds; fd_index++)
        {
            if( poll_set[fd_index].revents & POLLIN ) {
                if(poll_set[fd_index].fd == server_sockfd) {
                    client_len = sizeof(client_address);
                    client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, (socklen_t *)&client_len);


                    poll_set[numfds].fd = client_sockfd;
                    poll_set[numfds].events = POLLIN;
                    numfds++;


                    printf("Adding client on fd %d\n", client_sockfd);
                }
                else {
                    ioctl(poll_set[fd_index].fd, FIONREAD, &nread);


                    if( nread == 0 )
                    {
                        close(poll_set[fd_index].fd);
                        poll_set[fd_index].events = 0;
                        printf("Removing client on fd %d\n", poll_set[fd_index].fd);
                        int i = fd_index;
                        //for( i = fd_index; i<numfds; i++)
                        //{
                        poll_set[i] = poll_set[i + 1];
                        //}
                        numfds--;
                    }


                    else {
                        read(poll_set[fd_index].fd, &ch, 1);
                        printf("Serving client on fd %d\n", poll_set[fd_index].fd);
                        ch++;
                        write(poll_set[fd_index].fd, &ch, 1);
                    }
                }
            }




        }






    }
}

---

EDIT: I see that 'i' is no longer used, I have removed it and replaced it with fd_index in the actual code.

line 98 should be

Code:

---

poll_set[fd_index] = poll_set[numfds-1];

---

Quote:

---

Originally Posted by vart

line 98 should be

Code:

---

poll_set[fd_index] = poll_set[numfds-1];

---

---

Would that not have exactly the same effect as

Code:

---

poll_set[fd_index] = poll_set[fd_index + 1];

---

EDIT: in fact your suggestion gives broken pipe errors. what seems to work, in full, is

Code:

---


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


#define POLL_SIZE 32
#define LISTEN_QUEUE 5
#define SOCKET_PATH "web/server_socket.sock"


#define TRUE 1
#define FALSE 0




int main()
{
    int server_sockfd, client_sockfd;
    int server_len, client_len;
    struct sockaddr_un server_address;
    struct sockaddr_un client_address;
    int result;


    struct pollfd poll_set[POLL_SIZE];
    int numfds = 0;


    unlink(SOCKET_PATH);
    server_sockfd = socket(AF_UNIX, SOCK_STREAM, 0);


    server_address.sun_family = AF_UNIX;
    strcpy(server_address.sun_path, SOCKET_PATH);
    server_len = sizeof(server_address);


    bind(server_sockfd, (struct sockaddr *)&server_address, (socklen_t)server_len);
    listen(server_sockfd, 5);
    memset(poll_set, '\0', sizeof(poll_set));
    poll_set[0].fd = server_sockfd;
    poll_set[0].events = POLLIN;
    numfds++;
    //signal(SIGPIPE, SIG_IGN);






    while (1) {
        char ch;
        int fd_index;
        int nread;


        printf("Waiting for client (%d total)...\n", numfds);
        //result = select( (max_fd + 1), &testfds, (fd_set *)0, (fd_set *)0, (struct timeval *) 0);
        poll(poll_set, numfds, -1);
        for(fd_index = 0; fd_index < numfds; fd_index++)
        {
            if( poll_set[fd_index].revents & POLLIN ) {
                if(poll_set[fd_index].fd == server_sockfd) {
                    client_len = sizeof(client_address);
                    client_sockfd = accept(server_sockfd, (struct sockaddr *)&client_address, (socklen_t *)&client_len);


                    poll_set[numfds].fd = client_sockfd;
                    poll_set[numfds].events = POLLIN;
                    numfds++;


                    printf("Adding client on fd %d\n", client_sockfd);
                }
                else {
                    ioctl(poll_set[fd_index].fd, FIONREAD, &nread);


                    if( nread == 0 )
                    {
                        close(poll_set[fd_index].fd);
                        poll_set[fd_index].events = 0;
                        printf("Removing client on fd %d\n", poll_set[fd_index].fd);
                        //int i = fd_index;
                        //for( i = fd_index; i<numfds; i++)
                        //{
                        poll_set[fd_index] = poll_set[fd_index + 1];
                        //}
                        numfds--;
                    }


                    else {
                        read(poll_set[fd_index].fd, &ch, 1);
                        printf("Serving client on fd %d\n", poll_set[fd_index].fd);
                        ch++;
                        write(poll_set[fd_index].fd, &ch, 1);
                    }
                }
            }




        }






    }
}

---

it will work only if you are closing next to last socket

If you have sockets 1,2,3,4,5 and close number 2 you will have 1,3,3,4 and stop checking 5, I suggest to make it 1,5,3,4

How exactly would you implement this? Your suggestion earlier (

Code:

---

poll_set[fd_index] = poll_set[numfds-1];

---

) didn't work, some of the clients aren't closed properly giving broken pipe errors. The only method that has worked properly so far has been to shift all the elements along using a loop

Code:

---

int i = fd_index;
for( i = fd_index; i<numfds; i++)
{
                                poll_set[i] = poll_set[i + 1];
}
 numfds--;

---