0

Okay, i've searched the almighty google for some clear answear that would fit my problem but I was unsuccessfull. I'm developing an hardware abstraction layer in C++ that communicates via various serial protocols (SPI, I2C, UART), and I got a lot of ICs on the board that need controlling. Every IC gets it's own class. Since i'm modeling the hardware through classes, I think it's important for me to have the same ammount of instances in my code as the number of ICs mounted on the board. Here's my problem: I need to control the creation of these instances. The solution I came up with was to store the intances in a static std::map, which has a std::string as key (I use the device name for SPI, and the address for I2C for example). The code goes something like this:

IC.h

class SomeICMap { private: static std::map<std::string addr, std::shared_ptr<IC> > instance_map; public: static IC* getInitializedInstance(std::shared_ptr<CommInterface> comm, const std::string& addr); }

IC.cpp

std::map<std::string addr, std::shared_ptr<IC> > instance_map; IC* SomeICMap::getInitializedInstance(std::shared_ptr<CommInterface> comm, const std::string& addr) { std::map<string, std::shared_ptr<IC> >::iterator it; it = instance_map.find(addr); if (it == instance_map.end()) { std::shared_ptr<IC> device(new IC(comm, addr)); if (device->init() != 0) { return NULL; } instance_map[addr] = device; return device.get(); } return it->second.get(); }

This way I don't get duplicated instances of the hardware that is mounted on the board. I did this for every IC.

My question is: Is this thread safe?

I'm going to use some of these ICs in multiple threads running under Linux. I'm not sure if this is going to be safe, since I'm accesing an static map to get the pointers and access the hardware. From what I read online, the actual access to the hardware is safe, since the kernel takes care of concurrency when using write() read() to manipulate the open file descriptor. What I'm worried about is race conditions to occurr when the program first creates the IC instances.

Improve this question
1
  • stl containers are not thread safe. Commented Jun 26, 2014 at 18:06

2 Answers 2

Reset to default
1

SomeICMap::getInitializedInstance is not thread-safe: it's possible for some thread to access instance_map while another thread is modifying it. This is a data race, and C++ does not define behavior for programs with data races. You could solve this by throwing a mutex into the class and ensuring that all accesses to the map are performed while holding the mutex:

class SomeICMap { private: static std::map<std::string addr, std::shared_ptr<IC> > instance_map; static std::mutex mtx; // <------- public: static IC* getInitializedInstance(std::shared_ptr<CommInterface> comm, const std::string& addr); }; std::map<std::string addr, std::shared_ptr<IC> > SomeICMap::instance_map; std::mutex SomeICMap::mtx; // <------- IC* SomeICMap::getInitializedInstance(std::shared_ptr<CommInterface> comm, const std::string& addr) { std::lock_guard<std::mutex> lock(mtx); // <------- std::map<string, std::shared_ptr<IC> >::iterator it; it = instance_map.find(addr); if (it == instance_map.end()) { std::shared_ptr<IC> device(new IC(comm, addr)); if (device->init() != 0) { return NULL; } instance_map[addr] = device; return device.get(); } return it->second.get(); }
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

better a reader-writer lock, because creations are few and far between.
0

If you're calling getInitializedInstance from more than one thread concurrently, then no, this isn't threadsafe. The reason is that it would be possible for a map insertion to interleave with either another insertion or a read.

A simple solution would to add a static std::mutex to the class and construct a unique_lock around it at the start of the function.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.