Re[9]: Изобретен класс что-то типа Mutex'a. - WIN API

Привет всем!

Вот написал маленький класс и думаю не изобрел ли я велосипед (в который раз...

)

Как его обозвать даже не знаю, и как описать тоже, но попробую.

Итак, имеется у нас объект. Его можно Open и Release (по аналогии с mutex'oм). Открыть его можно только N раз, как только все его освободили то у нас устанавливается событие. Как только его кто-то открыл тоже устанавливается событие, но другое. Пока писал эти строки, пришло в голову, что N раз это не принципиально. Вот вообщем-то и все.
Можно пользовать в многопоточном приложении (для этого и задумывался), но есть сумнения. может вы, уважаемый алл, подскажите (выделенно жирным).

// - header ---
class CBlackMutex
{
  int Size;
  long ResCount;
  HANDLE hEvents[2];
public:
  __fastcall CBlackMutex(int sz=5);
  __fastcall ~CBlackMutex();
  bool __fastcall Open();
  bool __fastcall Release();

  HANDLE GetEmpty(){return hEvents[1];}
  HANDLE GetOpened(){return hEvents[0];}

};
// - code ---
__fastcall CBlackMutex::CBlackMutex(int sz)
{
  if (sz <= 0) sz = 1;
  Size = sz;
  ResCount = 0;
  hEvents[0] = hEvents[1] = NULL;
  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
}

__fastcall CBlackMutex::~CBlackMutex()
{
  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);
  hEvents[0] = hEvents[1] = NULL;
}

bool __fastcall CBlackMutex::Open()
{
  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
  {
    SetEvent(hEvents[0]);   // нет проверки ошибок...
  }
  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
  return true;
}
bool __fastcall CBlackMutex::Release()
{
  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
  {
    SetEvent(hEvents[1]);   // нет проверки ошибок...
  }
  return true;
}

Принимается любая критика по поводу кода.

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

BB>Привет всем!

BB> Вот написал маленький класс и думаю не изобрел ли я велосипед (в который раз...

)

BB> Как его обозвать даже не знаю, и как описать тоже, но попробую.

BB>Итак, имеется у нас объект. Его можно Open и Release (по аналогии с mutex'oм). Открыть его можно только N раз, как только все его освободили то у нас устанавливается событие. Как только его кто-то открыл тоже устанавливается событие, но другое. Пока писал эти строки, пришло в голову, что N раз это не принципиально. Вот вообщем-то и все.
BB>Можно пользовать в многопоточном приложении (для этого и задумывался), но есть сумнения. может вы, уважаемый алл, подскажите (выделенно жирным).

[покоцано]

BB>Принимается любая критика по поводу кода.

Любую критику, говорите, принимаете?

Итак:

1. Уберите от греха __fastcall. Тогда класс можно будет использовать в средах, отличных от Borland C++ Builder

.

2. По коду (убрал все __fastcall, ну и так, замечания по ходу):

// - header ---
class CBlackMutex
{
  int Size;
  long ResCount;
  HANDLE hEvents[2];
public:
  CBlackMutex(int sz=5); // лучше unsigned int
   ~CBlackMutex();
  bool Open();
  bool Release();

  HANDLE GetEmpty(){return hEvents[1];}
  HANDLE GetOpened(){return hEvents[0];}

};
// - code ---
CBlackMutex::CBlackMutex(unsigned int sz) // добавил unsigned 
{
  // если unsigned, то проверку можно убрать
//  if (sz <= 0) sz = 1;   

  Size = sz;
  ResCount = 0;
  hEvents[0] = hEvents[1] = NULL;
  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
}

CBlackMutex::~CBlackMutex()
{
// Тут я немножко подрезал код... И так все ясно, без != NULL

//  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
//  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);

  if ( hEvents[0] ) CloseHandle(hEvents[0]);
  if ( hEvents[1] ) CloseHandle(hEvents[1]);

// это зачем??? 
//  hEvents[0] = hEvents[1] = NULL; 
}

bool CBlackMutex::Open()
{
  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
  {
    SetEvent(hEvents[0]);   // нет проверки ошибок...
  }
  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
  return true;
}
bool CBlackMutex::Release()
{
  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
  {
    SetEvent(hEvents[1]);   // нет проверки ошибок...
  }
  return true;
}

Вот, в общем... Сами просили

BB>// - header ---
BB>class CBlackMutex
BB>{
BB>  int Size;
BB>  long ResCount;
BB>  HANDLE hEvents[2];
BB>public:
BB>  __fastcall CBlackMutex(int sz=5);
BB>  __fastcall ~CBlackMutex();
BB>  bool __fastcall Open();
BB>  bool __fastcall Release();
BB>
BB>  HANDLE GetEmpty(){return hEvents[1];}
BB>  HANDLE GetOpened(){return hEvents[0];}
BB>
BB>};
BB>// - code ---
BB>__fastcall CBlackMutex::CBlackMutex(int sz)
BB>{
BB>  if (sz <= 0) sz = 1;
BB>  Size = sz;
BB>  ResCount = 0;
BB>  hEvents[0] = hEvents[1] = NULL;
BB>  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
BB>  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
BB>}

BB>__fastcall CBlackMutex::~CBlackMutex()
BB>{
BB>  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
BB>  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);
BB>  hEvents[0] = hEvents[1] = NULL;
BB>}

BB>bool __fastcall CBlackMutex::Open()
BB>{
BB>  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
BB>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
BB>  {
BB>    SetEvent(hEvents[0]);   // нет проверки ошибок...
BB>  }
BB>  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
BB>  return true;
BB>}
BB>bool __fastcall CBlackMutex::Release()
BB>{
BB>  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
BB>  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
BB>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
BB>  {
BB>    SetEvent(hEvents[1]);   // нет проверки ошибок...
BB>  }
BB>  return true;
BB>}


BB>

BB>Принимается любая критика по поводу кода.

я не буду оценивать нужность класса и его функционал, а только попридираюсь к коду. ок?
1. нет конструктора по умолчанию — неудобно
2. зачем нужно

hEvents[0] = hEvents[1] = NULL;

в деструкторе
3. хачем для чтения ResCount использовать InterlockedExchangeAdd(&ResCount, 0)
4. вместо

InterlockedExchangeAdd(&ResCount, 1);

можно просто

InterlockedIncrement(&ResCount);

5. вместо

InterlockedExchangeAdd(&ResCount, -1);

можно просто

InterlockedDecrement(&ResCount);

6. никак не пойму внутреннего смысла функций

HANDLE GetEmpty(){return hEvents[1];}
HANDLE GetOpened(){return hEvents[0];}

по моему их надо было назвать так

BOOL IsEmptyEventValid();
BOOL IsOpenEventValid();

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

F>Здравствуйте BlackBox, Вы писали:
[skip]

F>1. Уберите от греха __fastcall. Тогда класс можно будет использовать в средах, отличных от Borland C++ Builder

.

ok

[skip]
F>CBlackMutex::CBlackMutex(unsigned int sz) // добавил unsigned
F>{
F> // если unsigned, то проверку можно убрать
F>// if (sz <= 0) sz = 1;
Спасибо, как то не думал об этом.
F> Size = sz;
F> ResCount = 0;
F> hEvents[0] = hEvents[1] = NULL;
F> hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
F> hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>}

F>CBlackMutex::~CBlackMutex()
F>{
F>// Тут я немножко подрезал код... И так все ясно, без != NULL
Дело привычки

F>// if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
F>// if (hEvents[1] != NULL) CloseHandle(hEvents[1]);

F> if ( hEvents[0] ) CloseHandle(hEvents[0]);
F> if ( hEvents[1] ) CloseHandle(hEvents[1]);

F>// это зачем???
F>// hEvents[0] = hEvents[1] = NULL;
Это у меня параноя такая. Зачем делаю сам не знаю. Но делаю...
F>}

[skip]

F>Вот, в общем... Сами просили

А по поводу GetOpened & GetEmpty???

Да и вообще такая штука нужна кому-нибудь?

Здравствуйте Алекс, Вы писали:

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

[skip]

А>я не буду оценивать нужность класса и его функционал, а только попридираюсь к коду. ок?
А>1. нет конструктора по умолчанию — неудобно

CBlackMutex(int sz=5); // <--- не оно?

А>2. зачем нужно
А>

А>hEvents[0] = hEvents[1] = NULL;
А>

А>в деструкторе
А>3. хачем для чтения ResCount использовать InterlockedExchangeAdd(&ResCount, 0)
чтобы получить значение не изменяя его.
[skip]
А>6. никак не пойму внутреннего смысла функций
А>

А>HANDLE GetEmpty(){return hEvents[1];}
А>HANDLE GetOpened(){return hEvents[0];}
А>

у меня в программе один поток ждет, когда либо объект освободится либо, когда его кто-нибудь откроет...

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

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

BB>>Привет всем!

BB>> Вот написал маленький класс и думаю не изобрел ли я велосипед (в который раз...

)

BB>> Как его обозвать даже не знаю, и как описать тоже, но попробую.

BB>>Итак, имеется у нас объект. Его можно Open и Release (по аналогии с mutex'oм). Открыть его можно только N раз, как только все его освободили то у нас устанавливается событие. Как только его кто-то открыл тоже устанавливается событие, но другое. Пока писал эти строки, пришло в голову, что N раз это не принципиально. Вот вообщем-то и все.
BB>>Можно пользовать в многопоточном приложении (для этого и задумывался), но есть сумнения. может вы, уважаемый алл, подскажите (выделенно жирным).

F>[покоцано]

BB>>Принимается любая критика по поводу кода.

F>Любую критику, говорите, принимаете?

Итак:

F>1. Уберите от греха __fastcall. Тогда класс можно будет использовать в средах, отличных от Borland C++ Builder

.

F>2. По коду (убрал все __fastcall, ну и так, замечания по ходу):

F>

F>// - header ---
F>class CBlackMutex
F>{
F>  int Size;
F>  long ResCount;
F>  HANDLE hEvents[2];
F>public:
F>  CBlackMutex(int sz=5); // лучше unsigned int
F>   ~CBlackMutex();
F>  bool Open();
F>  bool Release();

F>  HANDLE GetEmpty(){return hEvents[1];}
F>  HANDLE GetOpened(){return hEvents[0];}

F>};
F>// - code ---
F>CBlackMutex::CBlackMutex(unsigned int sz) // добавил unsigned 
F>{
F>  // если unsigned, то проверку можно убрать
F>//  if (sz <= 0) sz = 1;   

F>  Size = sz;
F>  ResCount = 0;
F>  hEvents[0] = hEvents[1] = NULL;
F>  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>}

F>CBlackMutex::~CBlackMutex()
F>{
F>// Тут я немножко подрезал код... И так все ясно, без != NULL

F>//  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
F>//  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);

F>  if ( hEvents[0] ) CloseHandle(hEvents[0]);
F>  if ( hEvents[1] ) CloseHandle(hEvents[1]);

F>// это зачем??? 
F>//  hEvents[0] = hEvents[1] = NULL; 
F>}

F>bool CBlackMutex::Open()
F>{
F>  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
F>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>  {
F>    SetEvent(hEvents[0]);   // нет проверки ошибок...
F>  }
F>  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
F>  return true;
F>}
F>bool CBlackMutex::Release()
F>{
F>  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
F>  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
F>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>  {
F>    SetEvent(hEvents[1]);   // нет проверки ошибок...
F>  }
F>  return true;
F>}

F>

F>Вот, в общем... Сами просили

И я тут поучаствую

CBlackMutex::~CBlackMutex()
{
// Я бы сделал во как
  if ( hEvents[0] != INVALID_HANDLE_VALUE) CloseHandle(hEvents[0]);
  if ( hEvents[1] != INVALID_HANDLE_VALUE ) CloseHandle(hEvents[1]);

  hEvents[0] = hEvents[1] = INVALID_HANDLE_VALUE; 
}

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

BB>Здравствуйте Алекс, Вы писали:

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

BB>[skip]

А>>я не буду оценивать нужность класса и его функционал, а только попридираюсь к коду. ок?
А>>1. нет конструктора по умолчанию — неудобно
BB>

BB>CBlackMutex(int sz=5); // <--- не оно?
BB>

А>>2. зачем нужно
А>>

А>>hEvents[0] = hEvents[1] = NULL;
А>>

А>>в деструкторе
А>>3. хачем для чтения ResCount использовать InterlockedExchangeAdd(&ResCount, 0)
BB>чтобы получить значение не изменяя его.
BB>[skip]
А>>6. никак не пойму внутреннего смысла функций
А>>

А>>HANDLE GetEmpty(){return hEvents[1];}
А>>HANDLE GetOpened(){return hEvents[0];}
А>>

BB>у меня в программе один поток ждет, когда либо объект освободится либо, когда его кто-нибудь откроет...

А зачем было hEvents[...] обьявлять как private, если потом так спокойно
их отдавать?

Здравствуйте Alex Smirnov, Вы писали:

[skip]

AS>А зачем было hEvents[...] обьявлять как private, если потом так спокойно
AS>их отдавать?

Да, конечно, их можно сделать и public, но как лучше всего сделать в многопоточной среде??
Или и так уже все нормально, и осталось только исправить private на public.

Здравствуйте Alex Smirnov, Вы писали:

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

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

BB>>>Привет всем!

BB>>> Вот написал маленький класс и думаю не изобрел ли я велосипед (в который раз...

)

BB>>> Как его обозвать даже не знаю, и как описать тоже, но попробую.

BB>>>Итак, имеется у нас объект. Его можно Open и Release (по аналогии с mutex'oм). Открыть его можно только N раз, как только все его освободили то у нас устанавливается событие. Как только его кто-то открыл тоже устанавливается событие, но другое. Пока писал эти строки, пришло в голову, что N раз это не принципиально. Вот вообщем-то и все.
BB>>>Можно пользовать в многопоточном приложении (для этого и задумывался), но есть сумнения. может вы, уважаемый алл, подскажите (выделенно жирным).

F>>[покоцано]

BB>>>Принимается любая критика по поводу кода.

F>>Любую критику, говорите, принимаете?

Итак:

F>>1. Уберите от греха __fastcall. Тогда класс можно будет использовать в средах, отличных от Borland C++ Builder

.

F>>2. По коду (убрал все __fastcall, ну и так, замечания по ходу):

F>>

F>>// - header ---
F>>class CBlackMutex
F>>{
F>>  int Size;
F>>  long ResCount;
F>>  HANDLE hEvents[2];
F>>public:
F>>  CBlackMutex(int sz=5); // лучше unsigned int
F>>   ~CBlackMutex();
F>>  bool Open();
F>>  bool Release();

F>>  HANDLE GetEmpty(){return hEvents[1];}
F>>  HANDLE GetOpened(){return hEvents[0];}

F>>};
F>>// - code ---
F>>CBlackMutex::CBlackMutex(unsigned int sz) // добавил unsigned 
F>>{
F>>  // если unsigned, то проверку можно убрать
F>>//  if (sz <= 0) sz = 1;   

F>>  Size = sz;
F>>  ResCount = 0;
F>>  hEvents[0] = hEvents[1] = NULL;
F>>  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>>  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>>}

F>>CBlackMutex::~CBlackMutex()
F>>{
F>>// Тут я немножко подрезал код... И так все ясно, без != NULL

F>>//  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
F>>//  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);

F>>  if ( hEvents[0] ) CloseHandle(hEvents[0]);
F>>  if ( hEvents[1] ) CloseHandle(hEvents[1]);

F>>// это зачем??? 
F>>//  hEvents[0] = hEvents[1] = NULL; 
F>>}

F>>bool CBlackMutex::Open()
F>>{
F>>  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
F>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>>  {
F>>    SetEvent(hEvents[0]);   // нет проверки ошибок...
F>>  }
F>>  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
F>>  return true;
F>>}
F>>bool CBlackMutex::Release()
F>>{
F>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
F>>  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
F>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>>  {
F>>    SetEvent(hEvents[1]);   // нет проверки ошибок...
F>>  }
F>>  return true;
F>>}

F>>

F>>Вот, в общем... Сами просили

AS>И я тут поучаствую

AS>

AS>CBlackMutex::~CBlackMutex()
AS>{
AS>// Я бы сделал во как
AS>  if ( hEvents[0] != INVALID_HANDLE_VALUE) CloseHandle(hEvents[0]);
AS>  if ( hEvents[1] != INVALID_HANDLE_VALUE ) CloseHandle(hEvents[1]);

AS>  hEvents[0] = hEvents[1] = INVALID_HANDLE_VALUE; 
AS>}
AS>

ну здесь ты не прав, т.к. в случае ошибки CreateEvent() возвращает NULL, а не INVALID_HANDLE_VALUE.

З.Ы.
INVALID_HANDLE_VALUE = -1
NULL = 0

BB>CBlackMutex(int sz=5); // <--- не оно?
BB>

сори, не заметил

BB>чтобы получить значение не изменяя его.

не понял?

Здравствуйте Алекс, Вы писали:

А>Здравствуйте Alex Smirnov, Вы писали:

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

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

BB>>>>Привет всем!

BB>>>> Вот написал маленький класс и думаю не изобрел ли я велосипед (в который раз...

)

BB>>>> Как его обозвать даже не знаю, и как описать тоже, но попробую.

BB>>>>Итак, имеется у нас объект. Его можно Open и Release (по аналогии с mutex'oм). Открыть его можно только N раз, как только все его освободили то у нас устанавливается событие. Как только его кто-то открыл тоже устанавливается событие, но другое. Пока писал эти строки, пришло в голову, что N раз это не принципиально. Вот вообщем-то и все.
BB>>>>Можно пользовать в многопоточном приложении (для этого и задумывался), но есть сумнения. может вы, уважаемый алл, подскажите (выделенно жирным).

F>>>[покоцано]

BB>>>>Принимается любая критика по поводу кода.

F>>>Любую критику, говорите, принимаете?

Итак:

F>>>1. Уберите от греха __fastcall. Тогда класс можно будет использовать в средах, отличных от Borland C++ Builder

.

F>>>2. По коду (убрал все __fastcall, ну и так, замечания по ходу):

F>>>

F>>>// - header ---
F>>>class CBlackMutex
F>>>{
F>>>  int Size;
F>>>  long ResCount;
F>>>  HANDLE hEvents[2];
F>>>public:
F>>>  CBlackMutex(int sz=5); // лучше unsigned int
F>>>   ~CBlackMutex();
F>>>  bool Open();
F>>>  bool Release();

F>>>  HANDLE GetEmpty(){return hEvents[1];}
F>>>  HANDLE GetOpened(){return hEvents[0];}

F>>>};
F>>>// - code ---
F>>>CBlackMutex::CBlackMutex(unsigned int sz) // добавил unsigned 
F>>>{
F>>>  // если unsigned, то проверку можно убрать
F>>>//  if (sz <= 0) sz = 1;   

F>>>  Size = sz;
F>>>  ResCount = 0;
F>>>  hEvents[0] = hEvents[1] = NULL;
F>>>  hEvents[0] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>>>  hEvents[1] = CreateEvent(NULL, FALSE, FALSE, NULL);
F>>>}

F>>>CBlackMutex::~CBlackMutex()
F>>>{
F>>>// Тут я немножко подрезал код... И так все ясно, без != NULL

F>>>//  if (hEvents[0] != NULL) CloseHandle(hEvents[0]);
F>>>//  if (hEvents[1] != NULL) CloseHandle(hEvents[1]);

F>>>  if ( hEvents[0] ) CloseHandle(hEvents[0]);
F>>>  if ( hEvents[1] ) CloseHandle(hEvents[1]);

F>>>// это зачем??? 
F>>>//  hEvents[0] = hEvents[1] = NULL; 
F>>>}

F>>>bool CBlackMutex::Open()
F>>>{
F>>>  if (InterlockedExchangeAdd(&ResCount, 0) == Size) return false;
F>>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>>>  {
F>>>    SetEvent(hEvents[0]);   // нет проверки ошибок...
F>>>  }
F>>>  InterlockedExchangeAdd(&ResCount, 1);           // multithreading ++ :D
F>>>  return true;
F>>>}
F>>>bool CBlackMutex::Release()
F>>>{
F>>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0) return false;
F>>>  InterlockedExchangeAdd(&ResCount, -1);          // multithreading -- :D
F>>>  if (InterlockedExchangeAdd(&ResCount, 0) == 0)
F>>>  {
F>>>    SetEvent(hEvents[1]);   // нет проверки ошибок...
F>>>  }
F>>>  return true;
F>>>}

F>>>

F>>>Вот, в общем... Сами просили

AS>>И я тут поучаствую

AS>>

AS>>CBlackMutex::~CBlackMutex()
AS>>{
AS>>// Я бы сделал во как
AS>>  if ( hEvents[0] != INVALID_HANDLE_VALUE) CloseHandle(hEvents[0]);
AS>>  if ( hEvents[1] != INVALID_HANDLE_VALUE ) CloseHandle(hEvents[1]);

AS>>  hEvents[0] = hEvents[1] = INVALID_HANDLE_VALUE; 
AS>>}
AS>>

А>ну здесь ты не прав, т.к. в случае ошибки CreateEvent() возвращает NULL, а не INVALID_HANDLE_VALUE.

А>З.Ы.
А>INVALID_HANDLE_VALUE = -1
А>NULL = 0

Сорри...Был не прав...Исправлюсь

Здравствуйте Алекс, Вы писали:

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

BB>>Здравствуйте Алекс, Вы писали:

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

BB>>[skip]

А>>>я не буду оценивать нужность класса и его функционал, а только попридираюсь к коду. ок?
А>>>1. нет конструктора по умолчанию — неудобно
BB>>

BB>>CBlackMutex(int sz=5); // <--- не оно?
BB>>

А>сори, не заметил

BB>>чтобы получить значение не изменяя его.

А>не понял?
InterlockedExchangeAdd возвращает предыдущее значение. Следовательно InterlockedExchangeAdd(&ResCount, 0) вернет нам значение ResCount, не изменив его. Может оно конечно можно и просто так прочитать, но лучше уж так...

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

хъ

А>>не понял?
BB>InterlockedExchangeAdd возвращает предыдущее значение. Следовательно InterlockedExchangeAdd(&ResCount, 0) вернет нам значение ResCount, не изменив его. Может оно конечно можно и просто так прочитать, но лучше уж так...

Дык чем лучше-то?

Здравствуйте Алекс, Вы писали:

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

А>хъ

А>>>не понял?
BB>>InterlockedExchangeAdd возвращает предыдущее значение. Следовательно InterlockedExchangeAdd(&ResCount, 0) вернет нам значение ResCount, не изменив его. Может оно конечно можно и просто так прочитать, но лучше уж так...

А>Дык чем лучше-то?

InterlockedExchangeAdd получает монопольный доступ к изменяемой переменной. Пока она не вернет нам значение никто его изменить не сможет.
Пока тебе писал, мысль родилась может весь код работающий с внутреннми переменными в критическую секцию засунуть? Так действительно будет более монопольный доступ.

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

BB>Здравствуйте Алекс, Вы писали:

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

А>>хъ

А>>>>не понял?
BB>>>InterlockedExchangeAdd возвращает предыдущее значение. Следовательно InterlockedExchangeAdd(&ResCount, 0) вернет нам значение ResCount, не изменив его. Может оно конечно можно и просто так прочитать, но лучше уж так...

А>>Дык чем лучше-то?

BB>InterlockedExchangeAdd получает монопольный доступ к изменяемой переменной. Пока она не вернет нам значение никто его изменить не сможет.

Ты думаешь, что пока ты читаешь значение переменной в регистр кто-то ее изменит? Однако!

Здравствуйте Алекс, Вы писали:

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

BB>>Здравствуйте Алекс, Вы писали:

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

А>>>хъ

А>>>>>не понял?
BB>>>>InterlockedExchangeAdd возвращает предыдущее значение. Следовательно InterlockedExchangeAdd(&ResCount, 0) вернет нам значение ResCount, не изменив его. Может оно конечно можно и просто так прочитать, но лучше уж так...

А>>>Дык чем лучше-то?

BB>>InterlockedExchangeAdd получает монопольный доступ к изменяемой переменной. Пока она не вернет нам значение никто его изменить не сможет.

А>Ты думаешь, что пока ты читаешь значение переменной в регистр кто-то ее изменит? Однако!

а ты её обьяви как volatile...компилятор её в регистры и не будет запихивать.

Chto to eto ochen pohozhe na semafor, esli net, chto ne hvataet v semafore ?

Товарищи, имейте совесть!
Зачем ради одной фразы цитировать весть предыдущий ответ, который тоже не блещет предответной обработкой?

Получите ноль.

Здравствуйте Kubyshev Andrey, Вы писали:

KA>Chto to eto ochen pohozhe na semafor, esli net, chto ne hvataet v semafore ?

Да название я немного неверное придумал. Это действительно больше похоже на семафор.
Различие в том, что мой объект переходит в свободное состояние дважды (когда полностью свободен и когда только что перешел из свободного состояния), тогда как семафор только, когда его кто-нибудь открывает (насколько я понимаю семафор).

Здравствуйте Хитрик Денис, Вы писали:

ХД>Товарищи, имейте совесть!
ХД>Зачем ради одной фразы цитировать весть предыдущий ответ, который тоже не блещет предответной обработкой?

ХД>Получите ноль.

да просто забыл!

извиняюсь.

Вообще лучше сделать так (если сервак совсем нагибается):
1. не показывать текст предыдущего сообщения при ответе
2. завести кнопочку, которая вставляет текст предыдущего сообщения

Это приведет к тому что:
1. будут вставлять цитаты только тогда, когда они действительно нужны
2. уменишит общий вес (в байтах) топика

3. все будут довольны

А кидаться нулями это не метод.

Здравствуйте Alex Smirnov, Вы писали:

[]

AS>а ты её обьяви как volatile...компилятор её в регистры и не будет запихивать.

объясняю популярно!
Когда ты сравниваешь ResCount с нулем компилятор генерит примерно такой код:

cmp ResCount,0    ;1
jnz not_zero

или

mov eax,ResCount  ;2
or eax,eax
jnz not_zero

При вызове функции будет примерно следующая картина:

push 0
push ResCount
call InterlockedExchangeAdd
or eax,eax
jnz not_zero

Панимаешь? И volatile здесь не при чем.

Чего напрасно спорить...Посмотри на прототип:

LONG InterlockedExchangeAdd (
  LPLONG volatile Addend,  // addend
  LONG Value               // increment value
);

Кстати и все другие процы этого типа...

Панимаешь!

Здравствуйте Alex Smirnov, Вы писали:

AS>Чего напрасно спорить...Посмотри на прототип:

ну посмотрел и че?

AS>Кстати и все другие процы этого типа...

???

AS>Панимаешь!

что ты мне хочешь объяснить?

Здравствуйте Алекс, Вы писали:

А>Здравствуйте Alex Smirnov, Вы писали:

AS>>Чего напрасно спорить...Посмотри на прототип:

А>ну посмотрел и че?

AS>>Кстати и все другие процы этого типа...

А>???

AS>>Панимаешь!

А>что ты мне хочешь объяснить?

ну раз в прототипе тебе говорят что принимаемый параметр volatile LONG*
, так и обьявляй её именно так!!! И вызов её немного отличается от твоего!
push 0
mov eax,dword ptr [this]
push eax
call dword ptr [__imp__InterlockedExchangeAdd@8 (42E27Ch)]
При учете того что:
class MyClass
{
volatile LONG lResCount;
};
и вызове: InterlockedExchangeAdd( &lResCount, 0);

Догоняешь суть???

)

В МСДН-е есть какой-то "улучшенный мьютекс", отикс (optix), кажется, они его назвали, может там чего полезного для твоего класса окажется.

BB>Принимается любая критика по поводу кода.

Твой код и без меня накомментировали...Да и так пойдёт, раз работает!

Мне вот только очень интересно назначение этого класса, можешь как-нибудь попроще применение объяснить, может, мне или ещё кому и пригодится?

Здравствуйте Alex Smirnov, Вы писали:

[]

AS>ну раз в прототипе тебе говорят что принимаемый параметр volatile LONG*
AS>, так и обьявляй её именно так!!! И вызов её немного отличается от твоего!
AS> push 0
AS> mov eax,dword ptr [this]
AS> push eax
AS> call dword ptr [__imp__InterlockedExchangeAdd@8 (42E27Ch)]
AS> При учете того что:
AS> class MyClass
AS> {
AS> volatile LONG lResCount;
AS> };
AS> и вызове: InterlockedExchangeAdd( &lResCount, 0);

AS>Догоняешь суть???

нет
из предыдущего сообщения
что ты мне хочешь объяснить?

Здравствуйте Алекс, Вы писали:

А>Здравствуйте Alex Smirnov, Вы писали:

А>[]

AS>>ну раз в прототипе тебе говорят что принимаемый параметр volatile LONG*
AS>>, так и обьявляй её именно так!!! И вызов её немного отличается от твоего!
AS>> push 0
AS>> mov eax,dword ptr [this]
AS>> push eax
AS>> call dword ptr [__imp__InterlockedExchangeAdd@8 (42E27Ch)]
AS>> При учете того что:
AS>> class MyClass
AS>> {
AS>> volatile LONG lResCount;
AS>> };
AS>> и вызове: InterlockedExchangeAdd( &lResCount, 0);

AS>>Догоняешь суть???

А>нет
А>из предыдущего сообщения
А>что ты мне хочешь объяснить?

Твой код:

push 0
push ResCount
call InterlockedExchangeAdd

Мой код:

push 0
mov         eax,dword ptr [this] ; Запихивание адреса переменной а не значения!
push eax
call InterlockedExchangeAdd

А теперь?

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

K>В МСДН-е есть какой-то "улучшенный мьютекс", отикс (optix), кажется, они его назвали, может там чего полезного для твоего класса окажется.
Я про этот optix у Рихтера видел. Если не ошибаюсь это не сколько не то, что мне надо было.

K>Мне вот только очень интересно назначение этого класса, можешь как-нибудь попроще применение объяснить, может, мне или ещё кому и пригодится?
Я его делал для синхронизации потоков.
Мне необходимо было 2 события:

объект полность освободился — в этом случае я выводил информацию, типа "нечего делать подождемс..."
и объект кто-то открыл — значит кто-то что-то делает, скажем об этом пользователю.

Если не понятно могу кусочек кода привести:

  for (;;)
  {
    DWORD dw = 0;
    dw = WaitForSingleObject(BlackMutex.GetOpened(), INFINITE);
    if (dw == WAIT_OBJECT_0)
    {
      if (close) break;
      mylog->write("// - new session -----------");
      dw = WaitForSingleObject(BlackMutex.GetEmpty(), INFINITE);
      switch(dw)
      {
        case WAIT_OBJECT_0:
        {
          if (close) return 0;
          mylog->write("// - end of session -----------");
          break;
        }
        case WAIT_FAILED:
        {
          mylog->write("// - nothing to plane ------------");
          break;
        }
        default:
        {
          mylog->write("in animate thread error occured");
          break;
        }
      }
    }
    else
    {
      mylog->write("critical error in animate thread.");
    }
  }
  return 0;

Правда код не идеален

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

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

K>>В МСДН-е есть какой-то "улучшенный мьютекс", отикс (optix), кажется, они его назвали, может там чего полезного для твоего класса окажется.
BB> Я про этот optix у Рихтера видел. Если не ошибаюсь это не сколько не то, что мне надо было.

K>>Мне вот только очень интересно назначение этого класса, можешь как-нибудь попроще применение объяснить, может, мне или ещё кому и пригодится?
BB> Я его делал для синхронизации потоков.
BB> Мне необходимо было 2 события:
BB>

BB>

объект полность освободился — в этом случае я выводил информацию, типа "нечего делать подождемс..."
BB>

и объект кто-то открыл — значит кто-то что-то делает, скажем об этом пользователю.
BB>

BB>Если не понятно могу кусочек кода привести:
BB>

BB>  for (;;)
BB>  {
BB>    DWORD dw = 0;
BB>    dw = WaitForSingleObject(BlackMutex.GetOpened(), INFINITE);
BB>    if (dw == WAIT_OBJECT_0)
BB>    {
BB>      if (close) break;
BB>      mylog->write("// - new session -----------");
BB>      dw = WaitForSingleObject(BlackMutex.GetEmpty(), INFINITE);
BB>      switch(dw)
BB>      {
BB>        case WAIT_OBJECT_0:
BB>        {
BB>          if (close) return 0;
BB>          mylog->write("// - end of session -----------");
BB>          break;
BB>        }
BB>        case WAIT_FAILED:
BB>        {
BB>          mylog->write("// - nothing to plane ------------");
BB>          break;
BB>        }
BB>        default:
BB>        {
BB>          mylog->write("in animate thread error occured");
BB>          break;
BB>        }
BB>      }
BB>    }
BB>    else
BB>    {
BB>      mylog->write("critical error in animate thread.");
BB>    }
BB>  }
BB>  return 0;

BB>

BB>Правда код не идеален

Тады так в классе и напиши:

 BOOL  IsOpened( )
 {
    return WaitForSingleObject( hEvents[1], INFINITY) == WAIT_OBJECT_0;
 }

 BOOL  IsClosed( )
 {
    return WaitForSingleObject( hEvents[0], INFINITY) == WAIT_OBJECT_0;
 }

и в коде:

  for (;;)
  {
    if(BlackMutex.IsOpened())
    {
      if (close) break;
      mylog->write("// - new session -----------");
      if(BlackMutex.IsClosed())
      {
          if (close) return 0;
          mylog->write("// - end of session -----------");
      }
      else mylog->write("in animate thread error occured");
    }
    else mylog->write("critical error in animate thread.");
  }

Вроде и короче будет

Здравствуйте Alex Smirnov, Вы писали:

хъ

AS>Твой код:
AS>

AS>push 0
AS>push ResCount
AS>call InterlockedExchangeAdd
AS>

AS>Мой код:
AS>

AS>push 0
AS>mov         eax,dword ptr [this] ; Запихивание адреса переменной а не значения!
AS>push eax
AS>call InterlockedExchangeAdd
AS>

AS>А теперь?

что теперь?
При чем здесь адрес переменной. Повторяю еще раз. Для чтения переменной не нужно пользоваться объектами синхронизации или функциями наподобие InterlockedExchangeAdd. Это не эффективно, не естественно и не необходимо.
Если ты этого не понимаешь, то я пас. Больше овечать тебе не буду и прикратим этот флейм. Видит бог, IT и еже с ним

, что я предпринимал все попытки объяснить уважаемому господину Смирнову неэффективность кода, все видели как я старался, однако, к сожалению, это не помогло.

Больше я отвечать ему не буду.

Здравствуйте Alex Smirnov, Вы писали:

AS>Тады так в классе и напиши:
AS>

AS> BOOL  IsOpened( )
AS> {
AS>    return WaitForSingleObject( hEvents[1], INFINITY) == WAIT_OBJECT_0;
AS> }

AS> BOOL  IsClosed( )
AS> {
AS>    return WaitForSingleObject( hEvents[0], INFINITY) == WAIT_OBJECT_0;
AS> }
AS>

Спасибо!!!
Вопрос — возвращать BOOL обязательно? А другой тип нельзя?

Здравствуйте Алекс, Вы писали:

Для чтения переменной не нужно пользоваться объектами синхронизации или функциями наподобие InterlockedExchangeAdd. Это не эффективно, не естественно и не необходимо.

То есть пользоваться этими функциями имеет смысл только при изменении переменной?

p.s. я только начал осваивать многопоточное программирование ...

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

BB>Здравствуйте Alex Smirnov, Вы писали:

AS>>Тады так в классе и напиши:
AS>>

AS>> BOOL  IsOpened( )
AS>> {
AS>>    return WaitForSingleObject( hEvents[1], INFINITY) == WAIT_OBJECT_0;
AS>> }

AS>> BOOL  IsClosed( )
AS>> {
AS>>    return WaitForSingleObject( hEvents[0], INFINITY) == WAIT_OBJECT_0;
AS>> }
AS>>

BB>Спасибо!!!
BB>Вопрос — возвращать BOOL обязательно? А другой тип нельзя?

Кнчно мжно! А смысл? Раз стоит INFINITY, то вроде всего два выхода:
Сработал эвент / Ошибка ожидания

Здравствуйте Алекс, Вы писали:

А>что теперь?
А>При чем здесь адрес переменной. Повторяю еще раз. Для чтения переменной не нужно пользоваться объектами синхронизации или функциями наподобие InterlockedExchangeAdd. Это не эффективно, не естественно и не необходимо.
А>Если ты этого не понимаешь, то я пас. Больше овечать тебе не буду и прикратим этот флейм. Видит бог, IT и еже с ним

А>Больше я отвечать ему не буду.

Да Блин!

Всё понятно про краткость получаемого кода! Только!
Если человек хочет использовать безопасный интерфейс изменения
счетчиков предоставленный производителем системы (OS),
то очень глупо предлогать на это плевать !
ГОСПОДИН АЛЕКС!

И насчет этой нити — действительно бесмысленно её продолжать!

Здравствуйте Alex Smirnov, Вы писали:

AS>Здравствуйте Алекс, Вы писали:

А>>что теперь?
А>>При чем здесь адрес переменной. Повторяю еще раз. Для чтения переменной не нужно пользоваться объектами синхронизации или функциями наподобие InterlockedExchangeAdd. Это не эффективно, не естественно и не необходимо.
А>>Если ты этого не понимаешь, то я пас. Больше овечать тебе не буду и прикратим этот флейм. Видит бог, IT и еже с ним

А>>Больше я отвечать ему не буду.

AS>Да Блин!

AS>Всё понятно про краткость получаемого кода! Только!
AS>Если человек хочет использовать безопасный интерфейс изменения
AS>счетчиков предоставленный производителем системы (OS),
AS>то очень глупо предлогать на это плевать !
AS>ГОСПОДИН АЛЕКС!

AS>И насчет этой нити — действительно бесмысленно её продолжать!

Читаем MSDN:

Simple reads and writes to properly-aligned 32-bit variables are atomic. In other words, when one thread is updating a 32-bit variable, you will not end up with only one portion of the variable updated; all 32 bits are updated in an atomic fashion. However, access is not guaranteed to be synchronized. If two threads are reading and writing from the same variable, you cannot determine if one thread will perform its read operation before the other performs its write operation.

Таким образом, приходим к выводу, что использование InterlockedExchangeAdd и т. п. для чтения значения 32-битного значения правильно выровненной переменной излишне. (Хотя для того, чтобы просто обозначить потенциальную проблему, например, на случай изменения типа на 64-битный, я бы написал функцию-обертку.)

InterlockedExchangeAdd тратит безумно много времени на такую простую операцию.

Кроме того, эта функция по-прежнему не решает проблем синхронизации. Т. е. если вообще возник такой вопрос — насчет считывания значения атомарным образом, — то значит, в этот же момент кто-то может в эту же переменную писать, а тогда встает следующий вопрос: а что мы считаем таким вот расчудесным-распрекрасным супер-атомарным образом? То что, может быть, кто-то когда-то как-то туда запишет? Что-то тут не так.

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

Вот вроде и умный человек сюда заглянул...

Вот посоветуйте, как лучше поступать? Доверять Господину Алекс-у, или доверять мелкософту...Вот МСДН:

Synchronization and Multiprocessor Issues
Applications may encounter problems when run on multiprocessor systems due to assumptions they make which are valid only on single-processor systems.

Thread Priorities
Consider a program with two threads, one with a higher priority than the other. On a single-processor system, the higher priority thread will not relinquish control to the lower priority thread because the scheduler gives preference to higher priority threads. On a multiprocessor system, both threads can run simultaneously, each on its own processor.

Applications should synchronize access to data structures to avoid race conditions. Code that assumes that higher priority threads run without interference from lower priority threads will fail on multiprocessor systems.

Memory Caching
When a processor writes to a memory location, the value is cached to improve performance. Similarly, the processor attempts to satisfy read requests from the cache to improve performance. Furthermore, processors begin to fetch values from memory before they are requested by the application. This can happen as part of speculative execution or due to cache line issues.

As a result, multiple processors can have different views of the system memory state because their caches are out of synch. For example, the following code is not safe on a multiprocessor system:

int iValue;
BOOL fValueHasBeenComputed = FALSE;
extern int ComputeValue();

void CacheComputedValue()
{
if (!fValueHasBeenComputed)
{
iValue = ComputeValue();
fValueHasBeenComputed = TRUE;
}
}

BOOL FetchComputedValue(int *piResult)
{
if (fValueHasBeenComputed)
{
*piResult = iValue;
return TRUE;
}
else
return FALSE;
}
There is a race condition in this code on multiprocessor systems because the processor that executes CacheComputedValue the first time may write fValueHasBeenComputed to main memory before writing iValue to main memory. Consequently, a second processor executing FetchComputedValue at the same time reads fValueHasBeenComputed as TRUE, because the new value of iValue is still in the first processor's cache.

Processors can be instructed to force their memory caches to agree with main memory with special instructions. Such instructions ensure that previous read and write requests have completed and are made visible to other processors, and to ensure that that no subsequent read or write requests have started. Examples are:

Functions which enter or leave critical sections.
Functions which signal synchronization objects.
Wait functions.
Interlocked functions
Consequently, the multiprocessor race condition above can be repaired as follows:

void CacheComputedValue()
{
if (!fValueHasBeenComputed) {
iValue = ComputeValue();
InterlockedExchange((LONG*)&fValueHasBeenComputed, TRUE);
}
}
The InterlockedExchange function ensures that the value of iValue is updated for all processors before the value of fValueHasBeenComputed is set to TRUE.

От:	BlackBox	---
Дата:	31.07.02 09:13
Оценка:	20 (2)

От:	Flamer	http://users.livejournal.com/_flamer_/
Дата:	31.07.02 09:29
Оценка:	9 (2)

От:	Алекс	http://wise-orm.com
Дата:	31.07.02 09:37
Оценка:

От:	BlackBox	---
Дата:	31.07.02 09:38
Оценка:

От:	BlackBox	---
Дата:	31.07.02 09:44
Оценка:

	От:	Kubyshev Andrey
	Дата:	31.07.02 14:46
	Оценка:

От:	Хитрик Денис	RSDN
Дата:	31.07.02 14:55
Оценка:

	От:	Alex Smirnov
	Дата:	01.08.02 07:53
	Оценка:	9 (1)