0

My code worked fine untill I added some mutexes to the interface IPackage class. Now in the Package class, when I try to store::IPackage P = *(new store::Package(args)) I get store::IPackage::IPackage(const store:IPackage&) (declared implicitly) Function cannot be referenced as it is a deleted function Taking the mutexes out fixes the issue.. I'm still very new to C++. Is there any way to make this work? I don't think I could explictly define IPackage constructor as it's meant to be "hidden" in the interface.

relevant code:

IPackage.h


    class IPackage {
    public:
        virtual ~IPackage() {}

        void SetTextDataPtr(std::vector<std::string>* _ptr);
        STORE_API std::vector<std::string>* GetTextDataPtr();
        std::mutex m_mutex_text_data_ptr;
        ...
        }
    ...

.

IPackage.cpp
void store::IPackage::SetTextDataPtr(std::vector<std::string>* _ptr) {
    std::lock_guard<std::mutex> lock(m_mutex_text_data_ptr);
    m_text_data_ptr = _ptr;
}
STORE_API std::vector<std::string>* store::IPackage::GetTextDataPtr() {
    //std::lock_guard<std::mutex> lock(m_mutex_text_data_ptr);
    return m_text_data_ptr;
}
...

.

Package.h
    class Package : public IPackage { //contains one .txt and one .jpg file
    public:
        Package(const std::filesystem::path &_parent_path, const std::string &_package_name, STORE_ERROR &_error);
    };
    ...
robinzero200
  • 53
  • 5
  • 3
    Unfortunately (and with good reason) `std::mutex` cannot be copied. – user4581301 Aug 11 '20 at 21:07
  • 3
    `store::IPackage P = *(new store::Package(args))` is incorrect and not needed. Just do `store::Package P{args}` - solves both your copying issue and memory leak. – SergeyA Aug 11 '20 at 21:09
  • would that work though? IPackage has only the default constructor, as the "real" package constructor contains lengthy code that should be hidden. – robinzero200 Aug 11 '20 at 21:14
  • I'm wondering if I could define the mutexes in Package instead, but then I will have to be carefull to lock them each time a package element is set/gotten – robinzero200 Aug 11 '20 at 21:15
  • 2
    Your main issue is not with mutexes. It looks like your design is broken. What's the point of having an instance of an interface class? – SergeyA Aug 11 '20 at 21:16
  • I think Sergey and I both missed that `I` in `IPackage` or the lack of it in `Package`. Oops. `store::IPackage *P = new store::Package(args);` might be closer to what you need, but look into using [`std::unique_ptr`](https://en.cppreference.com/w/cpp/memory/unique_ptr) to formally establish ownership. – user4581301 Aug 11 '20 at 21:16
  • With more details someone can suggest the best-fit solution. – user4581301 Aug 11 '20 at 21:19
  • in the Main that uses this library I need to acess generate Packages through the library funtion and acess their information. I thought an interface was meant to "hide" the real code that you don't want to share to get stolen. Sorry we were thrown head first into this class with a lot of new things and I've really been trying to understand. I can't make Package public (?) – robinzero200 Aug 11 '20 at 21:20
  • 2
    General rule of thumb: Keep the mutex as close to what it is protecting as possible `Package` is probably the best place to keep it. – user4581301 Aug 11 '20 at 21:21
  • Thanks, I will put the mutexes inside Package and try from there on. It hadn't occured to me mutex couldnt be copied but that sounds very logical. – robinzero200 Aug 11 '20 at 21:23
  • 1
    You may need to think through the use of the mutex. The pointer is protected in the set and again in the get, but anyone can replace, destroy or otherwise wreak havoc on it in between. If you need to protect an entire transaction, this may not be enough. – user4581301 Aug 11 '20 at 21:26
  • I don't really understand... m_text_data_ptr is private? – robinzero200 Aug 11 '20 at 21:29
  • 1
    That makes it a bit safer, but the class or a `friend` can still operate on it. Plus [the owner](https://stackoverflow.com/questions/49024982/what-is-ownership-of-resources-or-pointers) of the `vector` is not specified in the question. They could do nasty things to it (like go out of scope) as well. – user4581301 Aug 11 '20 at 21:37

0 Answers0