threads that not working to copy a file into queue in C++

Question

I'm trying to do a function that reads information from a file into a queue and then another function that reads the information from the queue to a file. Both functions are enabled by threads. The problem is that the first function does not store the information correctly and I do not know why? can anyone help me? This is the code:

void MessagesSender::saveMessages()
{
 std::string line = "";
 std::ifstream file(FILE_TO_READ);

if (file.is_open())
{
    while (std::getline(file, line))
    {
        mutReadToFile.lock();
        this->message.push(line);
        mutReadToFile.unlock();
    }
    file.close();
}
else
{
    std::cout << "Faild" << std::endl;
}
std::ofstream removeFile(FILE_TO_READ, std::ofstream::out | std::ofstream::trunc); //to delete the file
std::cout << "wait a minute..." << std::endl;
//std::this_thread::sleep_for(std::chrono::seconds(60));

}

and the other function:

void MessagesSender::sendMessages()
{
std::ofstream file;
std::vector<std::string> connectUsers;

connectUsers = this->getConnectUsers();
file.open(FILE_TO_WRITE, std::ios::app);

if (file.is_open())
{
    while (!this->message.empty())
    {
        for (int i = 0; i < connectUsers.size(); i++)
        {
            file << connectUsers[i] << ": " << this->message.front() << std::endl;
        }
        mutReadToFile.lock();
        this->message.pop();
        mutReadToFile.unlock();
    }
    file.close();
    std::cout << "Copy successfully" << std::endl;
}
else //If the file doesn't open
{
    std::cout << "faild :(" << std::endl;;
}

}

this is the menu:

int main()
{
MessagesSender mess;
mess.printMenu();

std::thread read(&MessagesSender::saveMessages, mess);
std::thread write(&MessagesSender::sendMessages, mess);

read.join();
write.join();
system("pause");
return 0;

}

Answer 1

Your threads use copies of the MessagesSender.

std::thread read(&MessagesSender::saveMessages, mess);
std::thread write(&MessagesSender::sendMessages, mess);

These bind by value. Use std::ref to prevent copying mess:

std::thread read(&MessagesSender::saveMessages, std::ref(mess));
std::thread write(&MessagesSender::sendMessages, std::ref(mess));

Alternatively, use pointers:

std::thread read(&MessagesSender::saveMessages, &mess);
std::thread write(&MessagesSender::sendMessages, &mess);

Live Demo

As always, let me add a live demo.

It also

• shows how to use lock_guard or unique_lock to use exception safe locking
• fixes the operations (empty() was a data race because it was outside the lock)
• uses C++11 ranged for
• makes error handling on the filestreams idiomatic
• FIXES the misleading comment on the truncation (no file is removed, otherwise, just std::remove it?)
• uses RAII for ifstream/ofstream instead of manual close()

There are "bigger" functional issues:

• I've ASSUMED that you want the messages to be repeated for all connected users, instead of popping each time.
• There's a race condition. If your sendMessages thread starts too soon, it will just exit because the queue is empty. This is by design in your code. I'll leave that as an exorcism for the reader.

I might be forgetting some

Live On Coliru

#include <fstream>
#include <iostream>
#include <mutex>
#include <queue>
#include <thread>
#include <vector>

static const std::string FILE_TO_READ = "input.txt";
static const std::string FILE_TO_WRITE = "output.txt";

struct MessagesSender {
    std::mutex _queueMutex;

    std::vector<std::string> getConnectUsers() {
        return { "john", "alice", "bob" };
    }
    void printMenu() { std::cout << "I don't know what's for dinner\n"; }
    void sendMessages();
    void saveMessages();

    std::queue<std::string> _messages;
};

void MessagesSender::saveMessages()
{
    std::string line;
    {
        std::ifstream file(FILE_TO_READ);

        while (std::getline(file, line)) {
            std::lock_guard lock(_queueMutex);
            _messages.push(line);
        }
        if (!file.good() && !file.eof()) {
            std::cout << "Faild" << std::endl;
        }
    } // closes file
    {
        std::ofstream removeFile(FILE_TO_READ, std::ios::trunc); // to empty the file
    }
    std::cout << "Input closed" << std::endl;
}

// and the other function:

void MessagesSender::sendMessages()
{
    std::ofstream file(FILE_TO_WRITE, std::ios::app);
    if (!file) {
        std::cout << "faild :(" << std::endl;
        return;
    }

    auto const connectUsers = MessagesSender::getConnectUsers();

    std::unique_lock<std::mutex> lock(_queueMutex);
    while (!_messages.empty())
    {
        auto current = _messages.front();
        _messages.pop();

        lock.unlock();

        for (const auto& user : connectUsers) {
            file << user << ": " << current << std::endl;
        }

        lock.lock();
    }
    std::cout << "Copy successfully" << std::endl;
}

// this is the menu:

int main()
{
    MessagesSender mess;
    mess.printMenu();

    std::thread read(&MessagesSender::saveMessages, &mess);
    std::thread write(&MessagesSender::sendMessages, &mess);

    read.join();
    write.join();
}