#include <fstream>
#include <iostream>
#include <mutex>
#include <string>
#include <thread>
#include <vector>
class Computation
{
private:
std::ofstream file;
std::mutex file_mutex;
public:
Computation()
{
file.open("test.txt", std::ios::app);
}
void write_to_file(const std::string &data)
{
std::unique_lock<std::mutex> lock(file_mutex);
file << data << std::endl;
}
void calculate()
{
write_to_file("42\n");
}
void startCalculation()
{
std::vector<std::thread> threads;
for (int i = 0; i < 4; i++)
{
threads.emplace_back(calculate, this);
}
for (auto &th : threads)
{
th.join();
}
}
};
int main()
{
Computation comp = Computation();
comp.startCalculation();
return 0;
}
#Is this a threadsafe way to write to a file?
languid ore
burnt flickerBOT
chrome edge
```cpp
#include <fstream>
#include <iostream>
#include <mutex>
#include <string>...
I would use std::jthread instead of std::thred
languid ore
well a) thats c++20, b) wasnt really the question ^^
chrome edge
well a) thats c++20, b) wasnt really the question ^^
Yeah but figured it'd make it a bit more robust
when do you apply the mutex on the file object?
languid ore
ya, thats true ; )
chrome edge
when do you apply the mutex on the file object?
?
languid ore
not sure what you mean
chrome edge
not sure what you mean
When do you put the lock on the std::ofstream object?
hushed jackal
it's a little awkward (at least in this reduced example) to be using unique_lock instead of a lock_guard
unique_lock is best used if you need to lock/unlock/transfer lock ownership
lock_guard is fine/good when you want to keep the lock throughout the scope of the lock_guard
as for whether this is thread safe, it is
languid ore
im still confused, a lock takes a mutex as a parameter so how would i put it on an object? I thought in this, i first lock the file_mutex if some other threads is trying to write to the file it has to wait until the mutex is "ready" again
hushed jackal
because the way you did it is fine and you can ignore the remark on "putting the lock on X object"
chrome edge
because the way you did it is fine and you can ignore the remark on "putting the...
Take that as me trying to understand the code
Not that he did it wrong
languid ore
ah so
const std::lock_guard<std::mutex> lock(g_i_mutex);
would have the same effect?
hushed jackal
well there is no g_i_mutex in your current piece of code
languid ore
ah yeah copied that streaight from cppref
hushed jackal
also locks are one of these things where you'd probably want to actually use {} even if you don't like them
I don't remember the exact example but there's a pretty common typo that makes it not work
languid ore
as an initializer or scope block?
hushed jackal
initializer
languid ore
const std::lock_guard<std::mutex> lock(file_mutex) = {};
okay great i guess, thanks!
hushed jackal
well no
languid ore
ok xd
hushed jackal
const std::lock_guard<std::mutex> lock{file_mutex};
languid ore
ah lmao
hushed jackal
but now that I'm trying to remember, the thing was more for unique_lock as it is default constructible, while lock_guard isn't
another common bug is something like std::lock_guard<std::mutex>{file_mutex};
which achieves nothing
and it's a tad too common if you start getting lazy with your variable name
I've seen people do std::lock_guard<std::mutex> g{file_mutex}; cause the "guard" name is kinda whatever and they want to go fast
then they typo it, skip the variable name, and end up back to std::lock_guard<std::mutex> {file_mutex};
which creates a temporary lock_guard that is immediately destructed
so iirc for lock_guard, paren is actually better (yes I'm contradicting myself)
because std::lock_guard<std::mutex> (file_mutex); would be a compile error
languid ore
1 step forward two back
hushed jackal
;compile -Wall -Wextra -Wpedantic
#include <mutex>
int main()
{
std::mutex my_mutex;
std::lock_guard<std::mutex> {my_mutex}; // warning maybe?
std::lock_guard<std::mutex> (my_mutex); // compile error, declaring variable with different type?
}
kindred fulcrumBOT
hushed jackal
so for lock_guard, I guess the temporary conclusion of me a sleep-deprived random passer-by alt-tabbing to your question is: use paren to avoid the typo error thing
and prefer lock_guard when lock guard works
because with unique_lock, std::unique_lock<std::mutex> (my_mutex); can sometime work
;compile -Wall -Wextra -Wpedantic
#include <mutex>
std::mutex g_mutex;
int main()
{
std::unique_lock<std::mutex> {g_mutex}; // temporary that gets immediately destroyed, maybe a warning sometime
std::unique_lock<std::mutex> (g_mutex); // default constructed object that shadows the global mutex
}
ok to wrap it up, i stick to
void write_to_file(const std::string &data)
{
const std::lock_guard<std::mutex> lock(file_mutex);
file << data << std::endl;
}
lock_guard here would be a compile error because it's not default constructible
yes
languid ore
great thanks for the many explanations and hopefully future bug preventions ; )
!solved
burnt flickerBOT
hushed jackal
I forgot one more, just to point out there's no silver bullet here
std::lock_guard<std::mutex> lock();
is a most vexing parse
so I guess it depends on whether you think it more likely to typo/forget the lock name or the mutex name
actually maybe auto here is closer to a silver bullet
auto lock = std::lock_guard<std::mutex>(g_mutex); and you can throw ctad on top
fierce hare
tbh, I kinda doubt that that's really what you wanna be doing.
writing to the same file from multiple threads, acquiring and releasing locks all the time… what exactly are you expecting to gain from this?
does the order in which things end up in the file really not matter?
also, note that C++20 introduced syncstreams