Является ли класс потокобезопастным?
От: wint  
Дата: 19.02.10 11:44
Оценка:
Здравствуйте.

У меня мало опыта написания многопоточных приложений, по этому хочу узнать мнение сообщества, является ли такой код потокобезопастным (thread safe):

Этот класс будет общим для всех потоков:
typdef boost::shared_ptr<Report> REPORT_PTR
typedef std::queue<REPORT_PTR>  REPORT_PTR_QUEUE
class ReportCollector
{
private:
    REPORT_PTR_QUEUE qReports;
    boost::mutex mLock;

public:
    ReportCollector() {};
    ~ReportCollector() {};
    void AddReport(REPORT_PTR& _report);
    void Show();
};

void ReportCollector::AddReport(REPORT_PTR &_report)
{
    boost::mutex::scoped_lock lock(mLock);
    
        qReports.push(_report);
}

void ReportCollector::Show()
{
    boost::mutex::scoped_lock lock(mLock);
    
    while (!qReports.empty())
    {
        REPORT_PTR r = qReports.front();
    std::wcout << L"Task result: " << r->result << std::endl;
        Reports.pop();
    }
}


Процедура обратного вызова для таймеров создающих отчеты:
void CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
{
    TimerRoutineArg* pArg = (TimerRoutineArg*)lpParam;
    
    std::vector<BYTE> vSenseInfo(1024);
    WORD wLength = 0;

    int result = OK_CODE;

    if (!pArg->vDllFuncArg.empty())
    {
        result= pArg->pSomeFunctionInDll(&(pArg->vDllFuncArg[0]), pArg->vDllFuncArg.size(), &vSenseInfo[0], wLength);
    }

    REPORT_PTR r(new Report());
    r->result = result;
    gReports.AddReport(r);
}


Процедура обратного вызова для таймера забирающего отчеты:
void CALLBACK ShowReportRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
{
    gReports.Show();
}


Функция Main:
// Глобальный объект (выше объявлен как extern ReportCollector gReports)
ReportCollector gReports;

void main()
{
    /* Создаем очередь таймеров и несколько таймеров с заданиями (TimerRoutine) */
    /* Создаем таймер с процедурой ShowReportRoutine */ 
   
    system("pause"); // Ждем пока не надоест.    
}
loading.............. 87%
Re: Является ли класс потокобезопастным?
От: remark Россия http://www.1024cores.net/
Дата: 19.02.10 11:52
Оценка:
Здравствуйте, wint, Вы писали:

W>У меня мало опыта написания многопоточных приложений, по этому хочу узнать мнение сообщества, является ли такой код потокобезопастным (thread safe):

Он потоко-безопасный, но не потоко-дружественный. Пока поток выводит отчёты, все остальные потоки стоят и не могут добавлять отчёты.



1024cores &mdash; all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re[2]: Является ли класс потокобезопастным?
От: wint  
Дата: 19.02.10 11:55
Оценка:
Здравствуйте, remark, Вы писали:

R>Здравствуйте, wint, Вы писали:

W>>У меня мало опыта написания многопоточных приложений, по этому хочу узнать мнение сообщества, является ли такой код потокобезопастным (thread safe):

R>Он потоко-безопасный, но не потоко-дружественный. Пока поток выводит отчёты, все остальные потоки стоят и не могут добавлять отчёты.



R>

А как сделать что был потоко-дружественным? (или где об этом почитать)
loading.............. 87%
Re[3]: Является ли класс потокобезопастным?
От: remark Россия http://www.1024cores.net/
Дата: 19.02.10 12:01
Оценка: +1
Здравствуйте, wint, Вы писали:

W>Здравствуйте, remark, Вы писали:

R>>Здравствуйте, wint, Вы писали:

W>>>У меня мало опыта написания многопоточных приложений, по этому хочу узнать мнение сообщества, является ли такой код потокобезопастным (thread safe):

R>>Он потоко-безопасный, но не потоко-дружественный. Пока поток выводит отчёты, все остальные потоки стоят и не могут добавлять отчёты.

W>А как сделать что был потоко-дружественным? (или где об этом почитать)


Навскидку, я бы сделал примерно так:

class ReportCollector
{
private:
    std::vector<Report*> qReports;
    std::vector<Report*> qReportsCache;
    boost::mutex mLock;

public:
    void AddReport(Reprot* _report)
    {
        boost::mutex::scoped_lock lock(mLock);
        qReports.push_back(_report);
    }

    void Show()
    {
        {
            boost::mutex::scoped_lock lock(mLock);
            qReports.swap(qReportsCache);
        }
        
        for (size_t i = 0; i != qReportsCache.size(); i += 1)
        {
            std::auto_ptr<Report> r (qReportsCache[i]);
            std::wcout << L"Task result: " << r->result << std::endl;
        }
        qReportsCache.clear();
    }

    ~ReportCollector()
    {
        for (size_t i = 0; i != qReports.size(); i += 1)
            delete qReports[i];
        for (size_t i = 0; i != qReportsCache.size(); i += 1)
            delete qReports[i];
    }
};

void CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
{
    ...

    std::auto_ptr<Report> r (new Report());
    r->result = result;
    gReports.AddReport(r.release());
}



1024cores &mdash; all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re[4]: Является ли класс потокобезопастным?
От: Кодт Россия  
Дата: 19.02.10 12:39
Оценка:
Здравствуйте, remark, Вы писали:

W>>А как сделать что был потоко-дружественным? (или где об этом почитать)
R>Навскидку, я бы сделал примерно так:

R>
R>class ReportCollector
R>{
R>private:
R>    std::vector<Report*> qReports;
R>    std::vector<Report*> qReportsCache;
.....
R>

А зачем слепок массива сделан членом класса?
Тем самым ты сломал потокобезопасность — точнее, засушил её до вида N producers 1 consumer. (Т.е. нельзя делать Show из двух потоков).
Ну и лишние данные в состояние класса занёс.

Есть смысл сделать qReportsCache локальной переменной метода Show.

Или это борьба за экономное использование кучи?
Тогда можно вот такой сон разума
class Collector
{
  the_vector queue_;
  the_vector spare_; // запчасть (пустой вектор с ненулевой ёмкостью)

  void Put(the_item i)
  {
    LOCKED { queue_.push_back(i); }
  }

  void Show()
  {
    // на входе: queue_:(qs,qc) (size, capacity)
    //           spare_:(0, sc)

    the_vector temp; // temp:(0,0)

    LOCKED
    {
      temp <-- queue_ <-- spare_ <-- temp; // temp.swap(queue_); queue_.swap(spare_);
    }
    // temp: (qs,qc); queue_: (0,sc); spare_(0,0)

    DoShow(temp);
    temp.resize(0);

    // temp:(0,qc); queue_:(qs1,qc1); spare_:(0,sc1)

    LOCKED
    {
      if(spare_.capacity() < temp.capacity()) // иначе смысла нет обмениваться
        spare_ <-- temp; // spare_.swap(temp);
    }
  }
}
Перекуём баги на фичи!
Re[5]: Является ли класс потокобезопастным?
От: remark Россия http://www.1024cores.net/
Дата: 19.02.10 13:01
Оценка:
Здравствуйте, Кодт, Вы писали:

К>А зачем слепок массива сделан членом класса?
К>Тем самым ты сломал потокобезопасность — точнее, засушил её до вида N producers 1 consumer. (Т.е. нельзя делать Show из двух потоков).
К>Ну и лишние данные в состояние класса занёс.
К>Есть смысл сделать qReportsCache локальной переменной метода Show.
К>Или это борьба за экономное использование кучи?


Идея — убрать ненужные обращения к менеджеру кучи и копирования. В устоявшемсе режиме оба контейнера доростут до максимального размера и просто будут обмениваться.
Насколько я понял задачу — поток, выводящий отчёт, один.


К>Тогда можно вот такой сон разума

Если выводящих потоков много, то тут тоже не будет устоявшегося режима — постоянно будут выделения и освобождения памяти.
Если требуется поддержка нескольких выводящих потоков, то я бы сделал так:


class ReportCollector
{
private:
    boost::mutex mLock;
    std::vector<Report*> qReports;

public:
    void AddReport(Reprot* _report)
    {
        boost::mutex::scoped_lock lock(mLock);
        qReports.push_back(_report);
    }

    void GetReports(std::vector<Report*>& reports)
    {
        reports.clear();
        boost::mutex::scoped_lock lock(mLock);
        qReports.swap(reports);
    }

    ~ReportCollector()
    {
        for (size_t i = 0; i != qReports.size(); i += 1)
            delete qReports[i];
    }
};



Дальше каждый поток волен сам кэшировать или не кэшировать пустой вектор.
Если используется кэширование, то потребление памяти в устоявшемсе режиме — (P+1)*N, P — кол-во потоков, N — кол-во отчётов за период.


1024cores &mdash; all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re[4]: Является ли класс потокобезопастным?
От: wint  
Дата: 21.02.10 17:00
Оценка:
Здравствуйте, remark, Вы писали:

R>...

R>Навскидку, я бы сделал примерно так:

R>
R>class ReportCollector
R>{
R>private:
R>    std::vector<Report*> qReports;
R>    std::vector<Report*> qReportsCache;
R>    boost::mutex mLock;

R>public:
R>    void AddReport(Reprot* _report)
R>    {
R>        boost::mutex::scoped_lock lock(mLock);
R>        qReports.push_back(_report);
R>    }

R>    void Show()
R>    {
R>        {
R>            boost::mutex::scoped_lock lock(mLock);
R>            qReports.swap(qReportsCache);
R>        }
        
R>        for (size_t i = 0; i != qReportsCache.size(); i += 1)
R>        {
R>            std::auto_ptr<Report> r (qReportsCache[i]);
R>            std::wcout << L"Task result: " << r->result << std::endl;
R>        }
R>        qReportsCache.clear();
R>    }

R>    ~ReportCollector()
R>    {
R>        for (size_t i = 0; i != qReports.size(); i += 1)
R>            delete qReports[i];
R>        for (size_t i = 0; i != qReportsCache.size(); i += 1)
R>            delete qReports[i];
R>    }
R>};

R>void CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
R>{
R>    ...

R>    std::auto_ptr<Report> r (new Report());
    r->>result = result;
R>    gReports.AddReport(r.release());
R>}

R>


Спасибо за рецепты. Выводящий поток один, поэтому пока остановился на этом варианте.
loading.............. 87%
Re[4]: Является ли класс потокобезопастным?
От: Юрий Жмеренецкий ICQ 380412032
Дата: 22.02.10 15:23
Оценка:
Здравствуйте, remark, Вы писали:

R>Навскидку, я бы сделал примерно так:

R>class ReportCollector
R>{
R>private:
R>    std::vector<Report*> qReports;
R>    std::vector<Report*> qReportsCache;
R>    boost::mutex mLock;

R>public:
R>    void AddReport(Reprot* _report)
R>    {
R>        boost::mutex::scoped_lock lock(mLock);
R>        qReports.push_back(_report);
R>    }
R>    
R>    //...    
R>};

R>void CALLBACK TimerRoutine(PVOID lpParam, BOOLEAN TimerOrWaitFired)
R>{
R>    ...

R>    std::auto_ptr<Report> r (new Report());
R>    r->result = result;
R>    gReports.AddReport(r.release());
R>}
R>


Небольшой оффтопик: выделенную строчку лучше заменить на две:

gReports.AddReport(r.get());
r.release();


иначе может быть утечка.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.