I'm working on a school project and I'm having trouble getting threading working with my classes. I have one class (Download) that has a thread that sets a pointer to an atomic<bool> true when the thread finishes work, and another class that is running on its own thread and when that one sees the status of a Download is complete, it should remove it from its list. With the class set up as it is now, any stores on any of my pointers to atomic<bool> will update the most recently created object only, so that only the end of the list is getting set to complete. Here's the code...
class Download
{
private:
Show *showPtr;
int season;
int episode;
int downloadTime;
atomic<bool> complete;
thread downloadThread;
// Edit: Hans Passant's solution to compiler error problems -
Download(const Download&);
Download& operator=(const Download&);
public:
Download(Show * showPtr);
string status(bool &complete);
bool operator==(const Download &other) const;
friend void blockThenSetComplete(Download *dl);
};
Download::Download(Show * showPtr)
: showPtr(showPtr)
{
complete.store(false);
// Randomly pick a download time from 20 - 30 seconds.
downloadTime = rand() % 11 + 20;
// Track which episode this thread is downloading.
season = showPtr->getNumberDownloaded() / showPtr->getNumberOfEpisodesPerSeason() + 1;
episode = showPtr->getNumberDownloaded() - showPtr->getNumberOfEpisodesPerSeason() * (season - 1) + 1;
showPtr->incrementDownloaded();
// Download the episode and return control.
downloadThread = thread(blockThenSetComplete, this);
}
string Download::status(bool & complete)
{
complete = this->complete.load();
stringstream ss;
ss << showPtr->getTitle() << " S" << season << "E" << episode;
return ss.str();
}
void blockThenSetComplete(Download *dl)
{
this_thread::sleep_for(chrono::seconds(dl->downloadTime));
dl->complete.store(true);
}
bool Download::operator==(const Download &other) const
{
if (other.showPtr == showPtr &&
other.season == season &&
other.episode == episode)
return true;
else
return false;
}
And the download manager...
// Ran as a thread, will fire up or tear down downloads as appropriate.
void downloadManager(Model *model)
{
while(true)
{
bool callNotify = false;
// monitoring shows
if (model->shows.size() != 0)
{
// connections available
if (model->account.getTotalConnectionCount() > model->account.getTotalActiveCount())
{
// check each show
for (list<Show>::iterator showIt = model->shows.begin(); showIt != model->shows.end(); showIt++)
{
// find a show that needs a download
if (~((*showIt).allDownloading()))
{
// must check connections again
if (model->account.getTotalConnectionCount() > model->account.getTotalActiveCount())
{
// Reserve connection and add to list.
model->account.reserveConnection();
model->downloads.push_back(Download(&(*showIt)));
callNotify = true;
}
}
}
}
}
// monitoring downloads
if (model->downloads.size() != 0)
{
// check each download
for (list<Download>::iterator downIt = model->downloads.begin(); downIt != model->downloads.end(); downIt++)
{
// find a finished download
bool finished = false;
(*downIt).status(finished);
if (finished)
{
// Remove from list, release connection, break as iterators are now invalid
model->downloads.remove(*downIt);
model->account.releaseConnection();
callNotify = true;
break;
}
}
}
if (callNotify)
model->notify();
// Check periodically.
this_thread::sleep_for(chrono::seconds(10));
}
}
I know there's no destructor for the pointers, that was getting called immediately after the thread was set off for some reason and making the program bail. The reason I'm using pointers is I can't seem to have threads or atomics as data members. Now that might be the real source of my problem. If I go through and change the pointers to instances of those classes, I get these errors:
Error 3 error C2248: 'std::atomic<bool>::atomic' : cannot access private member declared in class 'std::atomic<bool>' e:\users\robert\documents\visual studio 2012\projects\pvrsim\pvrsim\download.h 36 1 PVRsim
Error 4 error C2248: 'std::thread::thread' : cannot access private member declared in class 'std::thread' e:\users\robert\documents\visual studio 2012\projects\pvrsim\pvrsim\download.h 36 1 PVRsim
ANyone with more experience able to pick out what I'm doing wrong?
It is worth to take a look at the definition of the std::thread and std::atomic classes to see exactly what private member it is complaining about. When you peek at the vc/include/thr/thread header file for example, you'll see this:
private:
thrd_t _Thr;
bool _Joinable;
friend class thread_group;
thread(const thread&); // not defined
thread& operator=(const thread&); // not defined
Note the latter two members, respectively the copy constructor and the assignment operator. Note the // not defined comment.
This is a pretty standard trick in C++ to prevent client code from making a copy of the object of the class. Which is rather important for std::thread, there's no scenario where copying a thread can ever work. There is way too much runtime state associated with a thread, it has its own stack and its own execution state. No way that you can ever copy that. Or make that useful, modulo Unix' fork().
The part of this problem that's hard to guess at is why the compiler is trying to use the copy constructor. That's done in code that you cannot see. When you don't declare your own copy constructor and assignment operator then the C++ compiler will create one for you. Which will invoke the corresponding members of your member classes. Which are declared private, thus the compile error.
So what you need to do is prevent your own class from getting copied as well. Which is proper, it doesn't make sense to create a copy of your Download class object. You do so with the exact same technique that std::thread uses, make your copy constructor and assignment operator private and don't define them. Fix:
private:
Download(const Downoad&); // not defined
Download& operator=(const Download&); // not defined
The compiler will now no longer try to generate its own version of these members, problem solved.