0

So I am trying to work on this toy client file server problem.

The iterative version works fine, but when I attempt to fork each new connection in the server code, the client code hangs on the recv call.

I am fairly sure the issue is with the peer_socket, but I have been unable to find my mistake.

It completes just fine if I hit the server with a Ctrl-c though.

CLIENT CODE:

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <netinet/in.h>

#define PORT_NUMBER     5108
#define SERVER_ADDRESS  "localhost"
//#define FILENAME        "test.txt"

int main(int argc, char **argv)
{
    int client_socket;
    ssize_t len;
    struct sockaddr_in remote_addr;
    char buffer[BUFSIZ];
    char filename[256];
    char local_filename[256];
    int file_size;
    FILE *received_file;
    int remain_data = 0;

    /* Zeroing remote_addr struct */
    memset(&remote_addr, 0, sizeof(remote_addr));
    memset(local_filename, 0, sizeof(local_filename));
    if(argc >1){
        strncpy(local_filename, argv[1], 256);
    }
    /* Construct remote_addr struct */
    remote_addr.sin_family = AF_INET;
    inet_pton(AF_INET, SERVER_ADDRESS, &(remote_addr.sin_addr));
    remote_addr.sin_port = htons(PORT_NUMBER);

    /* Create client socket */
    client_socket = socket(AF_INET, SOCK_STREAM, 0);
    if (client_socket == -1)
    {
        fprintf(stderr, "Error creating socket --> %s\n", strerror(errno));

        exit(EXIT_FAILURE);
    }

    //Prompt for file to retrieve
    printf("Enter file name: ");
    fgets(filename,sizeof(filename),stdin);
    printf("Getting file %s", filename);



    /* Connect to the server */
    if (connect(client_socket, (struct sockaddr *)&remote_addr, sizeof(struct sockaddr)) == -1)
    {
        fprintf(stderr, "Error on connect --> %s\n", strerror(errno));

        exit(EXIT_FAILURE);
    }

    //Send filename to server
    /*if(!send(client_socket,filename, sizeof(filename))) {
        printf("Error sending filename\n");

        exit(EXIT_FAILURE);
    }*/
    send(client_socket,filename, sizeof(filename),0);
    printf("success\n");

    /* Receiving file size */
    memset(&buffer, 0, sizeof(buffer));
    recv(client_socket, buffer, sizeof(buffer), 0);
    file_size = atoi(buffer);
    fprintf(stdout, "\nFile size : %d\n", file_size);

    if(!local_filename[0]) {
        received_file = fopen("test2.txt", "w");
    }else{
        received_file = fopen(local_filename, "w");
    }
    if (received_file == NULL)
    {
        fprintf(stderr, "Failed to open file foo --> %s\n", strerror(errno));

        exit(EXIT_FAILURE);
    }

    remain_data = file_size;
//THIS IS THE LOOP THAT HANGS!
    while (((len = recv(client_socket, buffer, BUFSIZ, 0)) > 0) && (remain_data > 0))
    {
        fwrite(buffer, sizeof(char), len, received_file);
        remain_data -= len;
        fprintf(stdout, "Receive %d bytes and we hope :- %d bytes\n", len, remain_data);
    }
    printf("did we reach here in the code?\n");
    fclose(received_file);
    printf("did we reach here in the code?\n");
    close(client_socket);

    return 0;
}

SERVER CODE:

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <netinet/in.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/sendfile.h>
#include <signal.h>

#define PORT_NUMBER     5108
#define SERVER_ADDRESS  "localhost"
//#define FILE_TO_SEND    "hello.c"

int main(int argc, char **argv)
{
    int server_socket;
    socklen_t       global_sock_len;
    struct sockaddr_in      server_addr;
    struct sockaddr_in      peer_addr;

    /* Create server socket */
    server_socket = socket(AF_INET, SOCK_STREAM, 0);
    if (server_socket == -1)
    {
        fprintf(stderr, "Error creating socket --> %s", strerror(errno));

        exit(EXIT_FAILURE);
    }

    /* Zeroing server_addr struct */
    memset(&server_addr, 0, sizeof(server_addr));
    /* Construct server_addr struct */
    server_addr.sin_family = AF_INET;
    inet_pton(AF_INET, SERVER_ADDRESS, &(server_addr.sin_addr));
    server_addr.sin_port = htons(PORT_NUMBER);

    /* Bind */
    if ((bind(server_socket, (struct sockaddr *)&server_addr, sizeof(struct sockaddr))) == -1)
    {
        fprintf(stderr, "Error on bind --> %s", strerror(errno));

        exit(EXIT_FAILURE);
    }

    /* Listening to incoming connections */
    if ((listen(server_socket, 5)) == -1)
    {
        fprintf(stderr, "Error on listen --> %s", strerror(errno));

        exit(EXIT_FAILURE);
    }

    //attempt multiple connections via fork

    int peer_socket;
    int fid;
    while(1) {

        bool servexit = false;
        printf("did we reach HERE in the code?%d\n",getpid());
        peer_socket = accept(server_socket, (struct sockaddr *) &peer_addr, &global_sock_len);
        //printf("did we reach HERE in the code?\n");
        if (peer_socket == -1) {
            fprintf(stderr, "Error on accept --> %s", strerror(errno));

            exit(EXIT_FAILURE);
        }
        if((fid = fork()) == -1) {
            printf("error\n");
            close(peer_socket);
            //continue;
        }else if(fid > 0){
            printf("parent\n");
            close(peer_socket);
            //continue;
        }else if(fid == 0) {
            printf("child\n");
            socklen_t       sock_len;
            ssize_t len;
            int fd;
            int sent_bytes = 0;
            char FILE_TO_SEND[256];
            char file_size[256];
            struct stat file_stat;
            off_t offset;
            int remain_data;
            //int peer_socket = peer_socket;
            recv(peer_socket, FILE_TO_SEND, sizeof(FILE_TO_SEND), 0);
            printf("Client requested %s", FILE_TO_SEND);
            if (strlen(FILE_TO_SEND) > 1) {
                strtok(FILE_TO_SEND, "\n");
            }

            //check for server close command from client
            if(strcmp(FILE_TO_SEND,"quit") == 0){
                servexit = true;
                printf("Quiting server\n");
                close(server_socket);
                close(peer_socket);
                pid_t pid = getppid();
                kill(pid, SIGKILL);
                exit(0);
            }else {

                fd = open(FILE_TO_SEND, O_RDONLY);
                if (fd == -1) {
                    fprintf(stderr, "Error opening file --> %s", strerror(errno));

                    exit(EXIT_FAILURE);
                }

                /* Get file stats */
                if (fstat(fd, &file_stat) < 0) {
                    fprintf(stderr, "Error fstat --> %s", strerror(errno));

                    exit(EXIT_FAILURE);
                }

                fprintf(stdout, "File Size: \n%d bytes\n", file_stat.st_size);

                sock_len = sizeof(struct sockaddr_in);
                fprintf(stdout, "Accept peer --> %s\n", inet_ntoa(peer_addr.sin_addr));

                sprintf(file_size, "%d", file_stat.st_size);

                /* Sending file size */
                len = send(peer_socket, file_size, sizeof(file_size), 0);
                if (len < 0) {
                    fprintf(stderr, "Error on sending greetings --> %s", strerror(errno));

                    exit(EXIT_FAILURE);
                }

                fprintf(stdout, "Server sent %d bytes for the size\n", len);
                //fflush((FILE*)peer_socket);
                offset = 0;
                remain_data = file_stat.st_size;
                /* Sending file data */
                while (((sent_bytes = sendfile(peer_socket, fd, &offset, BUFSIZ)) > 0) && (remain_data > 0)) {
                    fprintf(stdout,
                            "1. Server sent %d bytes from file's data, offset is now : %d and remaining data = %d\n",
                            sent_bytes, offset, remain_data);
                    remain_data -= sent_bytes;
                    fprintf(stdout,
                            "2. Server sent %d bytes from file's data, offset is now : %d and remaining data = %d\n",
                            sent_bytes, offset, remain_data);
                }

            }
            break;
        }

    }
    return 0;

}
  • 1
    The parent should close the peer socket after forking. Otherwise, the client will never get EOF when the child process exits. – Barmar Aug 11 '17 at 20:31
  • For some reason you have that line commented out. Uncomment it. – Barmar Aug 11 '17 at 20:32
  • Thanks, I realize that those lines should be uncommented, but they weren't affecting my particular problem (yet). Turns out that the issue was that sendfile() was acting differently than send() In a way I hadn't expected. What fixed it was retrying the recv() calls in client if a -1(error) was returned rather than exiting. – a_river_in_canada Aug 11 '17 at 20:43
  • `int sent_bytes` and `int len` are the wrong type to use to hold the return values from `send()` and `sendfile()`. `sendfile()` and `send()` both return `ssize_t` and not `int`. `sprintf(file_size, "%d", file_stat.st_size);` is wrong, see https://stackoverflow.com/questions/586928/how-should-i-print-types-like-off-t-and-size-t for the correct `printf()`-style format for an `off_t`. Using the proper types is important. What you've used will fail miserably if you run into files larger than 2 GB and/or if you compile your code on a 64-bit platform. – Andrew Henle Aug 11 '17 at 21:16
  • The first problem to be fixed it that neither the client nor the server code cleanly compiles. Infact the server code has some undefined names that it uses, like `false` and 'true' So the server code is missing the statement: `#include ` – user3629249 Aug 12 '17 at 02:58
  • when compiling, always enable all the warnings, then fix those warnings. For instance, when the parameters to `main()` are not used, the the signature should be: `int main( void )` – user3629249 Aug 12 '17 at 02:59
  • before calling `exit()`, the code should always clean up after itself. I.E. close open files, – user3629249 Aug 12 '17 at 03:02
  • regarding: `if (strlen(FILE_TO_SEND) > 1) { strtok(FILE_TO_SEND, "\n"); }` this code block is making the assumption that every line in the input file ends with a '\n' This is not a valid assumption even for a .txt file. – user3629249 Aug 12 '17 at 03:04
  • the name of the file to send (may) end with a '\n', and if it does, then that '\n' needs to be replaced with a NUL (0x00) character. Using `strtok()` to do so is misleading and a very poor method to use/ – user3629249 Aug 12 '17 at 03:07
  • regarding this line: `len = send(peer_socket, file_size, sizeof(file_size), 0);` there is no guarantee that all of a desired packet will be sent in the first call to `send()` so should be comparing the returned value 'len' with the desired size: `sizeof(file_size)` and looping if not all the packet is sent, trying to send the remaining part of the packet, until all of it is succesfully sent. – user3629249 Aug 12 '17 at 03:11
  • the posted code contains some 'magic' numbers. 'magic' numbers are numbers with no basis. I.E. 256. 'magic' numbers make the code much more difficult to understand, debug, etc. Suggest using a `enum` statement or a `#define` statement to give those 'magic' numbers meaningful names, then using those meaningful names throughout the code – user3629249 Aug 12 '17 at 03:14
  • when calling `recv()`, always check the returned value for 0==EOF and <0==error occurred. – user3629249 Aug 12 '17 at 03:15
  • regarding: `printf("error\n");` When a system function returns an error indication 1) the error message should be output to `stderr`, not `stdout`. 2) should include the systems reason for the error. Suggest using `perror();` to accomplish those two functions. – user3629249 Aug 12 '17 at 03:18
  • regarding: `pid_t pid = getppid(); kill(pid, SIGKILL);` why would the code want to kill the server if the child process fails? Also, the server process is never calling `wait()` or `waitpid()` on any of the child processes. So if some child kills the parent, (the server process) then any/all child processes will become 'zombies' Which are very difficult to eliminate without rebooting the computer. Suggest using a thread that can perform the wait() or waitpid() calls – user3629249 Aug 12 '17 at 03:25
  • Note: the child processes never exit. When they are done communicating with a client, they should call `exit();` – user3629249 Aug 12 '17 at 03:28
  • regarding: `len = send(peer_socket, file_size, sizeof(file_size), 0);` this should be sending the size of the file to be sent, NOT sending an extra, uninitialized 225 (or so bytes) after sending the file size integer. BTW: what if the file to send is greater than 2gig in size? – user3629249 Aug 12 '17 at 03:33
  • the predefined variable: BUFSIZE is defined as 4096 in stdio.h, which does not match the actual buffer size of 256. So the code is accessing far beyond the bounds of the buffer, This is undefined behavior and can lead to a seg fault event. – user3629249 Aug 12 '17 at 03:36
  • when calling: `fgets()`, always check the returned value to assure the operation was successful – user3629249 Aug 12 '17 at 03:41
  • this line: `send(client_socket,filename, sizeof(filename),0);` will (usually) be sending a lot of initialized data to the server, suggest only sending (at max) strlen( filename)+1 – user3629249 Aug 12 '17 at 03:43
  • this statement: `if(!local_filename[0]) {` will always evaluate as 'true' because the `local_filename[]` array was initialized to all 0x00 and never set otherwise. – user3629249 Aug 12 '17 at 03:46
  • when calling `fwrite()` should be checking the returned value. It could have failed, the disk could be full, and several other errors. – user3629249 Aug 12 '17 at 03:49
  • The posted code, in the client, should be using 'setsockoption()` to have te socket stay open for more than one communication sequence. – user3629249 Aug 12 '17 at 03:58

1 Answers1

-1

Ok, so, it turns out that sendfile() doesn't play as nicely as send() in this situation. I found two fixes.

1.) Use send() instead of sendfile()

2.) Retry in both sendfile() (Server) and recv() (client) if either returns -1 (error)