Failed to read the same file using multithread

Question

I created 50 threads to read the same file at the same time and then, in each thread, tried to write its content to new file that create with different name. The code was supposed to generate 50 different files. But I got unexpected results that it just generate 3~5 files.

When all the read the same file, there is no race-condition, and each thread is aimed to write its content to different file.

Can somebody help me? Thank you!

My code is listed below and it is a modification from Reference

#include <unistd.h>
#include <stdio.h>
#include <fcntl.h>
#include <stdlib.h>
#include <sys/stat.h>
#include <string.h>
#include <iostream>
#include <vector>
#include <thread>

void copy(const char *src_path, const char *dst_path);

int main(int argc, char **argv)
{
    std::vector<std::thread> vth;
    char *tmp = "Copy.deb";
    for (int i = 0; i < 50; ++i)
    {
        char src[40];
        memset(src, '\0', sizeof(src));
        sprintf(src, "%d", i);
        strcat(src, tmp);
        vth.emplace_back(std::bind(copy, "Original.deb", strdup(src)));
    }
    for (int i = 0; i < 50; ++i)
    {
        vth[i].join();
    }

    return 0;
}

void copy(const char *src_path, const char *dst_path)
{
    FILE *src, *dst;
    int buffer_size = 8 * 1024;
    char buffer[buffer_size];
    size_t length;

    src = fopen(src_path, "rb");
    dst = fopen(dst_path, "wb");

    while (!feof(src))
    {
        length = fread(buffer, 1, buffer_size, src);
        fwrite(buffer, 1, length, dst);
    }

    fclose(src);
    fclose(dst);
}

Answer 1

I believe your problem is that you are passing src (which is a pointer to a local variable on your main thread's stack) to your thread's entry function, but since your copy() function runs asynchronously in a separate thread, the char src[40] array that you are passing a pointer to has already been been popped off of the main thread's stack (and likely overwritten by other data) before the copy() function gets a chance to read its contents.

The easy fix would be to make a copy of the string on the heap, so that you can guarantee the string will remain valid until the the copy() function executes and reads it:

    vth.emplace_back(std::bind(copy, "Original.deb", strdup(src)));

... and be sure to have your copy() function free the heap-allocation when it's done using it:

void copy(const char *src_path, const char *dst_path)
{
  FILE *src, *dst;
  int buffer_size = 8 * 1024;
  char buffer[buffer_size];
  size_t length;

  src = fopen(src_path, "rb");
  dst = fopen(dst_path, "wb");

  free(dst_path);   // free the string previously allocated by strdup()

  [...]

Note that you don't currently have the same problem with the "Original.deb" argument since "Original.deb" is a string-literal and therefore stored statically in the executable, which means it remains valid for as long as the program is running -- but if/when you change your code to not use a string-literal for that argument, you'd likely need to do something similar for it as well.