Why the program here exemplifying the usage of named pipe -FIFO in LINUX suffers from race condition?

StackOverflow https://stackoverflow.com/questions/21498439

  •  05-10-2022
  •  | 
  •  

سؤال

I have two programs - one program writes to the pipe, and the other program reads from the pipe. However, when I run them, the messages are missed.

writer program is the following -

The writing process when executed -

$ ./pipe.out 
Write to Buffer = 0

The reading process when executed -

$ ./pipe_reader.out 
Pipe already exists
Read from Buffer
0
Read from Buffer
7
Read from Buffer
7
Read from Buffer

Did you see that the messages are lost here?

Here is the program, please explain what is the problem here? The same works if I don't close the File descriptor. In fact, keep opening it. I am sure this will lead to some other problem. Here, it works since the fd opened are just 10.

Program - Class

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


using namespace std;


class Named_Pipe
{

private:

int m_fp;
char m_name[256];
int m_rd;
int m_wr;

public:

void open_pipe(char *name);
void close_reader();
void close_writer();
void write_to_pipe(char *msg);
void read_from_pipe(char *msg);
Named_Pipe();

};

Named_Pipe::Named_Pipe()
{

  /** no access to any groups and others **/
  umask(077);

}

void Named_Pipe::open_pipe(char *name)
{ 
    int rc;
    struct stat pipe_exist;

    strcpy(m_name,name);    

    if ( stat(name, &pipe_exist) == 0 ) 
    {

      cout<<"Pipe already exists"<<endl;    
    }

    else
    {

    rc = mkfifo(name, S_IRWXU); 

     if( rc == -1)
     {
      cout<<strerror(rc);   

     }
    }   

}

void Named_Pipe::write_to_pipe(char *msg)
{
    int rc = 0;

    m_wr = open(m_name,O_WRONLY);

    if( m_wr == -1)
    {
      cout<<"Error in opening the descriptor for writing"<<endl;    
    }

    rc = write(m_wr,msg,256);

    if( rc == -1)
    cout<<"Error in writing the message"<<endl;

}

void Named_Pipe::read_from_pipe(char *msg)
{
    int rc = 0;

    m_rd = open(m_name,O_RDONLY);

    if( m_rd == -1)
    {
      cout<<"Error in opening the descriptor for reading"<<endl;    
    }

    rc = read(m_rd,msg,256);

    if( rc == -1)
    cout<<"Error in reading the message"<<endl;

}

void Named_Pipe:: close_reader()
{
   close(m_rd); 

}

void Named_Pipe:: close_writer()
{
   close(m_wr); 

}

Now, the writer pipe process logic -

pipe.cpp

int main(int argc, char *argv[])
{
  Named_Pipe write_process;
  char buffer[256];
  int i = 0;

  write_process.open_pipe("FIRST_FIFO_PROG");

  for( i= 0; i<10; i++)
  {

   strcpy(buffer,"MY FIRST MSG ON PIPES");

   cout<<"Write to Buffer = "<< i<< endl;
   sprintf(buffer,"%d",i);

   write_process.write_to_pipe(buffer);
   write_process.close_writer();


  }




  return 0; 
}

Now, the reader pipe process here.

int main(int argc, char *argv[])
{
  Named_Pipe read_process;
  char buffer[256];
  int i = 0;

  read_process.open_pipe("FIRST_FIFO_PROG");

  for( i= 0; i<10; i++)
  {

   cout<<"Read from Buffer"<<endl;

   read_process.read_from_pipe(buffer);
   cout<<buffer<<endl;
   read_process.close_reader();   

  }

  return 0; 
}
هل كانت مفيدة؟

المحلول

You keep opening and closing the FIFO on both the read and write sides. You only need to open it once, write (and read) your messages, and then close the FIFO.

What you are seeing is not so much a race condition as a timing problem of your own making. A FIFO needs both a reader and writer before the respective open calls succeed and a FIFO can have multiple readers and writers.

What I see are variations on the following:

  1. reader & writer open
  2. writer writes
  3. writer closes
  4. Steps 2&3 loop x times before...
  5. reader reads a single message
  6. reader closes and the remainder of the messages are lost.

Also you write a fixed 256 bytes where you probably want to write strlen bytes. And, as always, read & write in a loop.

نصائح أخرى

Please do not accept this answer — accept Duck's answer.


Analysis

Your class member functions actions do not match their names.

  • The constructor simply sets umask() and does nothing else. It doesn't even set the file descriptors to a known state.
  • The open_pipe() function creates the FIFO if there is no file of that name.
  • The write_to_pipe() function opens the file each time it is called and writes to the pipe without then closing it.
  • The read_from_pipe() function opens the file each time it is called and reads from the pipe without then closing it.
  • The close_reader() and close_writer() functions close the file descriptor regardless of whether the corresponding read or write has been executed.
  • There is no destructor.
  • The m_fp member variable of the class is never used.

Despite this litany of problems, the code superficially should work — except that a FIFO discards any written data when all the file descriptors are closed. You got this key information from the comment by Duck.

Note that if your code does more than one read or write before calling close, you are leaking resources horribly.

Here's a rewrite — dealing with some of the design issues.

pipe.h

#ifndef PIPE_H_INCLUDED
#define PIPE_H_INCLUDED

class Named_Pipe
{
private:
    char m_name[256];
    int m_rd;
    int m_wr;
    bool m_mk;      // FIFO created?
    void mkpipe(char const *name);

public:
    void open_reader(const char *name);
    void open_writer(const char *name);
    void close_reader();
    void close_writer();
    void write_to_pipe(const char *msg);
    int read_from_pipe(char *msg, int maxlen);

    Named_Pipe();
    ~Named_Pipe();
};

#endif // PIPE_H_INCLUDED

pipe.cpp

#include "pipe.h"

#include <cerrno>
#include <cstring>
#include <cstdlib>
#include <fcntl.h>
#include <iostream>
#include <sys/stat.h>
#include <unistd.h>

using namespace std;

Named_Pipe::Named_Pipe() : m_rd(-1), m_wr(-1), m_mk(false)
{
    m_name[0] = '\0';
    umask(077);
}

Named_Pipe::~Named_Pipe()
{
    close_reader();
    close_writer();
    unlink(m_name);
}

void Named_Pipe::mkpipe(char const *name)
{
    struct stat pipe_exist;

    strcpy(m_name, name);

    if (stat(name, &pipe_exist) != 0)
    {
        if (mkfifo(name, S_IRWXU) != 0)
        {
            cerr << strerror(errno);
            exit(1);
        }
    }
    m_mk = true;
}

void Named_Pipe::open_reader(char const *name)
{
    if (!m_mk)
        mkpipe(name);
    m_rd = open(m_name, O_RDONLY);
    if (m_rd == -1)
    {
        cerr << "Error in opening " << name << " for reading" << endl;
        exit(1);
    }
}

void Named_Pipe::open_writer(char const *name)
{
    if (!m_mk)
        mkpipe(name);
    m_wr = open(m_name, O_WRONLY);
    if (m_wr == -1)
    {
        cerr << "Error in opening FIFO " << name << " for writing" << endl;
        exit(1);
    }
}

void Named_Pipe::write_to_pipe(char const *msg)
{
    if (m_wr == -1)
    {
        cerr << "Writing to unopened FIFO\n";
        exit(1);
    }
    int len = strlen(msg) + 1;
    if (write(m_wr, msg, len) != len)
    {
        cerr << "Error in writing the message" << endl;
        exit(1);
    }
}

int Named_Pipe::read_from_pipe(char *msg, int msglen)
{
    if (m_rd == -1)
    {
        cerr << "Reading from unopened FIFO\n";
        exit(1);
    }
    int rc = read(m_rd, msg, msglen - 1);
    if (rc == -1)
        cerr << "Error in reading the message" << endl;
    else if (rc == 0)
        cerr << "EOF on pipe" << endl;
    else if (msg[rc-1] != '\0')
        msg[rc] = '\0';
    cerr << "Read " << rc << " bytes from FIFO\n";
    return rc;
}

void Named_Pipe::close_reader()
{
    if (m_rd != -1)
    {
        close(m_rd);
        m_rd = -1;
    }
}

void Named_Pipe::close_writer()
{
    if (m_wr != -1)
    {
        close(m_wr);
        m_wr = -1;
    }
}

pipe-reader.cpp

#include "pipe.h"
#include <iostream>
#include <cstring>

using namespace std;

int main()
{
    Named_Pipe read_process;
    char buffer[256];
    int i = 0;

    read_process.open_reader("FIRST_FIFO_PROG");

    for (i = 0; i < 10; i++)
    {
        int nbytes = read_process.read_from_pipe(buffer, sizeof(buffer));
        const char *data = buffer;
        int counter = 0;
        while (nbytes > 0)
        {
            int len = strlen(data);
            cout << "Reader" << counter << ": [" << data << "]" << endl;
            nbytes -= len + 1;
            data += len + 1;
        }
    }

    read_process.close_reader();
    cout << "Reader complete\n";

    return 0;
}

pipe-writer.cpp

#include "pipe.h"
#include <iostream>
#include <cstring>

using namespace std;

int main()
{
    Named_Pipe write_process;
    char buffer[256];

    write_process.open_writer("FIRST_FIFO_PROG");

    for (int i = 0; i < 10; i++)
    {
        sprintf(buffer, "Message on FIFO %d", i);
        cout << "Write to Buffer = [" << buffer << "]" << endl;
        write_process.write_to_pipe(buffer);
    }

    write_process.close_writer();
    cout << "Writer complete\n";

    return 0;
}

Sample outputs

Example 1:

$ pipe-writer & sleep 1 ; pipe-reader
[1] 9576
Write to Buffer = [Message on FIFO 0]
Write to Buffer = [Message on FIFO 1]
Write to Buffer = [Message on FIFO 2]
Write to Buffer = [Message on FIFO 3]
Write to Buffer = [Message on FIFO 4]
Write to Buffer = [Message on FIFO 5]
Write to Buffer = [Message on FIFO 6]
Write to Buffer = [Message on FIFO 7]
Write to Buffer = [Message on FIFO 8]
Write to Buffer = [Message on FIFO 9]
Writer complete
Read 180 bytes from FIFO
Reader0: [Message on FIFO 0]
Reader1: [Message on FIFO 1]
Reader2: [Message on FIFO 2]
Reader3: [Message on FIFO 3]
Reader4: [Message on FIFO 4]
Reader5: [Message on FIFO 5]
Reader6: [Message on FIFO 6]
Reader7: [Message on FIFO 7]
Reader8: [Message on FIFO 8]
Reader9: [Message on FIFO 9]
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
Reader complete
[1]+  Done                    pipe-writer
$

Example 2:

$ pipe-writer & pipe-reader
[1] 9579
Write to Buffer = [Message on FIFO 0]
Write to Buffer = [Message on FIFO 1]
Write to Buffer = [Message on FIFO 2]
Write to Buffer = [Message on FIFO 3]
Write to Buffer = [Message on FIFO 4]
Write to Buffer = [Message on FIFO 5]
Read Write to Buffer = [Message on FIFO 6]
Write to Buffer = [Message on FIFO 7]
Write to Buffer = [Message on FIFO 8]
Write to Buffer = [Message on FIFO 9]
36Writer complete
 bytes from FIFO
Reader0: [Message on FIFO 0]
Reader1: [Message on FIFO 1]
Read 144 bytes from FIFO
Reader0: [Message on FIFO 2]
Reader1: [Message on FIFO 3]
Reader2: [Message on FIFO 4]
Reader3: [Message on FIFO 5]
Reader4: [Message on FIFO 6]
Reader5: [Message on FIFO 7]
Reader6: [Message on FIFO 8]
Reader7: [Message on FIFO 9]
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
EOF on pipe
Read 0 bytes from FIFO
Reader complete
[1]+  Done                    pipe-writer
$

Now you can see why there's the extra loop in the reader main program.


I still don't regard this is as a good design. There should be just one file descriptor in the class. The name of the FIFO should be passed into the constructor, along with an indication of whether this is for reading or writing. The name of the FIFO is replicated in the two programs; a common name should be used. I kept the close_xxxxer() functions, but it would be better to let the destructor do the job. You would simplify the function names to read() and write(). The code error checks but writes to cerr (not cout) and exits on error. Note that the return code of -1 is not the same as the error number got from errno.

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


using namespace std;


class Named_Pipe
{

private:

int m_fp;
char m_name[256];
int m_rd;
int m_wr;

public:

void open_pipe(char *name);
void close_reader();
void close_writer();
void write_to_pipe(char *msg);
void read_from_pipe(char *msg);
Named_Pipe(int read_desc, int write_desc);
~Named_Pipe();

};

Named_Pipe::Named_Pipe(int read_desc, int write_desc):m_rd(read_desc), m_wr(write_desc)
{

  /** no access to any groups and others **/

  umask(077);

}

Named_Pipe::~Named_Pipe()
{
   /** This is to remove the pipe create **/
   unlink(m_name);   
}

void Named_Pipe::open_pipe(char *name)
{ 
    int rc;
    struct stat pipe_exist;

    strcpy(m_name,name);    

    if ( stat(name, &pipe_exist) == 0 ) 
    {

      cout<<"Pipe already exists"<<endl;    
    }

    else
    {

    rc = mkfifo(name, S_IRWXU); 

     if( rc == -1)
     {
      cout<<strerror(rc);   

     }
    }   

}

void Named_Pipe::write_to_pipe(char *msg)
{
    int rc = 0;

    if ( m_wr == -1)
    {

      m_wr = open(m_name,O_WRONLY);

      if( m_wr == -1)
      {
      cout<<"Error in opening the descriptor for writing"<<endl;    
      }

    }

    rc = write(m_wr,msg,256);

    if( rc == -1)
    cout<<"Error in writing the message"<<endl;

}

void Named_Pipe::read_from_pipe(char *msg)
{
    int rc = 0;

    if( m_rd != 0)
    {
     m_rd = open(m_name,O_RDONLY);

     if( m_rd == -1)
     {
      cout<<"Error in opening the descriptor for reading"<<endl;    
     }

    }

    rc = read(m_rd,msg,256);

    if( rc == -1)
    cout<<"Error in reading the message"<<endl;

}

void Named_Pipe:: close_reader()
{
   close(m_rd); 

}

void Named_Pipe:: close_writer()
{
   close(m_wr); 
}

Now, writer and reader process respectively.

int main(int argc, char *argv[])
{
  /* reader and writer , -1 un-initialized and 0 means intialized **/   
  Named_Pipe write_process(0,-1);
  char buffer[256];
  int i = 0;

  write_process.open_pipe("FIRST_FIFO_PROG");

  for( i= 0; i<10; i++)
  {

   sprintf(buffer,"%s %d","MY FIRST MSG ON PIPES",i);
   cout<<"Write Buffer = "<< buffer<< endl;
   write_process.write_to_pipe(buffer);

  }

   write_process.close_writer();

   return 0;    
}

Reader process-

int main(int argc, char *argv[])
{
  /* reader and writer , -1 un-initialized and 0 means intialized **/   
  Named_Pipe read_process(-1,0);
  char buffer[256];
  int i = 0;

  read_process.open_pipe("FIRST_FIFO_PROG");

  for( i= 0; i<10; i++)
  {

   read_process.read_from_pipe(buffer);
   cout<<"Read Buffer = "<< buffer<< endl;

  }

  read_process.close_reader();   

  return 0; 
}
مرخصة بموجب: CC-BY-SA مع الإسناد
لا تنتمي إلى StackOverflow
scroll top