std::lock_guard with variadic templates

Question

[It is not necessary to follow the links to understand the question].

I combined the implementation of the singleton pattern in this answer, together with the synchronized file writing of this other answer.

Then I wanted to see if the interface of SynchronizedFile could provide a variadic templated write method, but I couldn't figure out how to properly combine this with the std::lock_guard.

Below is a non-working example. In this case it doesn't work because (I think) the two threads manage to pump stuff into the buffer i_buf in a non-synchronized way, resulting in a garbled LOGFILE.txt.

If I put the std::lock_guard inside the general template of write then the program doesn't halt.

#include <iostream>
#include <mutex>
#include <sstream>
#include <fstream>
#include <string>
#include <memory>
#include <thread>

static const int N_LOOP_LENGTH{10};

// This class manages a log file and provides write method(s)
// that allow passing a variable number of parameters of different
// types to be written to the file in a line and separated by commas.
class SynchronizedFile {
public:
    static SynchronizedFile& getInstance()
    {
        static SynchronizedFile instance;
        return instance;
    }

private:
    std::ostringstream i_buf;
    std::ofstream i_fout;
    std::mutex _writerMutex;

    SynchronizedFile () {
        i_fout.open("LOGFILE.txt", std::ofstream::out);
    }

public:
    SynchronizedFile(SynchronizedFile const&) = delete;
    void operator=(SynchronizedFile const&)   = delete;

    template<typename First, typename... Rest>
    void write(First param1, Rest...param)
    {
        i_buf << param1 << ", ";
        write(param...);
    }

    void write()
    {
        std::lock_guard<std::mutex> lock(_writerMutex);
        i_fout << i_buf.str() << std::endl;
        i_buf.str("");
        i_buf.clear();
    }
};

// This is just some class that is using the SynchronizedFile class
// to write stuff to the log file.
class Writer {
public:
    Writer (SynchronizedFile& sf, const std::string& prefix) 
        : syncedFile(sf), prefix(prefix) {}

    void someFunctionThatWritesToFile () {
        syncedFile.write(prefix, "AAAAA", 4343, "BBBBB", 0.2345435, "GGGGGG");
    }
private:
    SynchronizedFile& syncedFile;
    std::string prefix;
};

void thread_method()
{
  SynchronizedFile &my_file1 = SynchronizedFile::getInstance();
  Writer writer1(my_file1, "Writer 1:");
  for (int i = 0; i < N_LOOP_LENGTH; ++ i)
    writer1.someFunctionThatWritesToFile();
}

int main()
{
  std::thread t(thread_method);

  SynchronizedFile &my_file2 = SynchronizedFile::getInstance();
  Writer writer2(my_file2, "Writer 2:");
  for (int i = 0; i < N_LOOP_LENGTH; ++i)
    writer2.someFunctionThatWritesToFile();

  t.join();
  std::cout << "Done" << std::endl;
  return 0;
}

How could I successfully combine these three ideas?

tkausl · Accepted Answer · 2019-03-05T23:15:18.690

2

The program deadlocks because write calls itself recursively while still holding the lock.

Either use a std::recursive_mutex ~~or release the lock after writing your data out but before calling write~~. E: Unlocking doesn't do the job, I didn't think this through...

E: Or lock once and defer to another private method to do the write.

template<typename... Args>
void write(Args&&... args)
{
    std::unique_lock<std::mutex> lock(_writerMutex);
    _write(std::forward<Args>(args)...);
}

template<typename First, typename... Rest>
void _write(First&& param1, Rest&&... param) // private method
{
    i_buf << std::forward<First>(param1) << ", ";
    _write(std::forward<Rest>(param)...);
}

void _write()
{
    i_fout << i_buf.str() << std::endl;
    i_buf.clear();
}

edited Mar 05 '19 at 23:15

answered Mar 05 '19 at 23:00

tkausl

13,686
2
33
50

Let me study the recursive_mutex (I am clearly new to working with threads). I didn't understand the positions that you are referring to in the second alternative. And you answered it already. – user647486 Mar 05 '19 at 23:03
@user647486 I've added an example. For `std::recursive_mutex` the `unlock` wouldn't be necessary. – tkausl Mar 05 '19 at 23:05
Some of these ideas might help me solve the problem. However, let me tell you that it looks like the first one with `unique_lock` didn't. I think it is because the `std::endl` is in a part that is not "locked". Some lines in the file get printed concatenated or after a couple of newline characters. Let me try the `recursive_mutex` approach. – user647486 Mar 05 '19 at 23:10
The `recursive_mutex` approach does make it work properly. Thank you. I didn't know about it. – user647486 Mar 05 '19 at 23:15

std::lock_guard with variadic templates

1 Answers1