Critical Section vs. Mutex Object vs. Interlocked Access

[RedPuma]

Wieso sollte man sich immer alles selbst programmieren?
Etwas selbst zu machen, nur weil es geht, ist meist schlecht. Wenn man es macht um dabei zu lernen, OK. Wenn man es nur macht weil man einen furchtbaren Dickschädel hat, ist es aber fast immer ein Fehler.

* Die Boost.Thread ist quasi-Standard, und wird fast 1:1 in der Form wohl auch im neuen C++ Standard landen
* Die Boost.Thread ist "tried and true", und von Leuten programmiert die ziemlich gut wissen was sie tun
* Die Boost.Thread ist komfortabel zu handhaben, was ich von deiner Klasse nicht sagen würde. Es fehlt z.B. die "Scoped-Lock" Klasse, die die Mutex/CRITICAL_SECTION im Konstruktor lockt und im Destruktor unlockt

Und noch ganz konkret zum Thema warum ich es für eine schlechte Idee halte, sich selbst was zu basteln, was es schon (als allgemein akzeptierte Komponente) fertig gibt...
Die meisten Leute machen bei der Implementierung nicht nur Fehler, sondern gewöhnen sich dabei auch an, bestimmte Dinge nach ihrem eigenen Dickschädel zu machen, und nicht wie sie "alle anderen" machen. Reicht schon wenn etwas was überall "foo" heisst bei dir dann "bar" heisst. Der Effekt der Implementierungsfehler dürfte klar sein. Der Effekt von anderen Namen/Begriffen ist, dass man anfängt ein anderes Vokabular zu verwenden. Und das ist IMO immer ein Nachteil, da es eine unnötige Kommunikations-Barriere darstellt.

Konkret am Beispiel deines Codes:

Implementierungs-Fehler:

* Deine CThreadMutex Klasse ist copy-constructable, dürfte es aber nicht sein

* Deine CThreadMutex Klasse ist assignable, dürfte es aber nicht sein

Quality-Of-Implementation:

* Der Konstruktor verwendet InitializeCriticalSection. InitializeCriticalSection kann fehlschlagen, diesen Umstand aber nicht kommunizieren. -> Lieber InitializeCriticalSectionAndSpinCount verwenden

* Du bietest keine Scoped-Lock Klasse an

* Du bietest keine Try-Lock Funktion an

Du hast natürlich völlig recht wenn es dabei um einen industriellen Maßstab geht. Dass meine Wrapper-Klasse vollständig oder besser als die boost-library ist habe ich nicht behauptet, dass sie fehlerfrei ist oder sonstiges auch nicht. Die sollte sowieso nur ein Beispiel sein wie einfach man sowas basteln kann. Bei der Einfachheit hätte ich z.B. gar keine Wrapper-Klasse genommen, jedoch kann ich bei meiner Version wie gesagt noch zwischen Mutex und Crit Section umschalten.

Dass die boost library inzwischen standard ist ist mir auch klar, nur finde ich eher dass man bei solch "kleinen" Problemen keine solch "riesige" Bibliothek heranziehen muss, und sich mit deren Benutzung auseinandersetzen muss. Vor allem in dem Hinblick dass sich der Eröffner dieses Threads schon zahlreich mit den Windows-Synchronisations-Objekten vertraut gemacht hat, und das Einarbeiten in boost::thread unnötig währe.

/Edit: Ach ja, der Name CThreadMutex stammt daher dass mich Visual Studio 10 keine Klasse mit dem Namen CMutex erstellen ließ. Also zumindest der Dialog nach der Schaltfläche "Klasse hinzufügen" verweigerte mir dies. Da ich mir dachte das dies vielleicht aus gutem Grund geschieht habe ich meine Klasse dann nicht einfach über den Texteditor erstellt sondern umbenannt.

[FrEEzE2046]

RedPuma schrieb:

Also zumindest der Dialog nach der Schaltfläche "Klasse hinzufügen" verweigerte mir dies. Da ich mir dachte das dies vielleicht aus gutem Grund geschieht habe ich meine Klasse dann nicht einfach über den Texteditor erstellt sondern umbenannt.

Das benutzt jemand? Das vorangestellte "C" sieht man sonst vor allem beim - mir zu widerem - MFC.

Desweiteren möchte ich die boost Library nicht heranziehen. Das wäre auch unnötig, da es sich hier um eine relativ kleine Problematik handelt für die man prinzipiell nicht mal eine Thread-Wrapper Klasse benötigt.

[FrEEzE2046]

Evtl. habe ich irgendetwas - event objects betreffend - noch nicht richtig verstanden. Kann mir jemand sagen warum ich hier beim Aufruf von WaitForSingleObject() in einer Endlosschleife hängen bleibe?

#include <iostream>
#include <process.h>
#include <Windows.h>

using namespace std;

unsigned __stdcall ThreadProc(void*);

int main()
{
	unsigned threadID = 0;
	DWORD	 threadExitCode = 0;

	HANDLE hEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr);
	HANDLE hThread = reinterpret_cast<HANDLE>(_beginthreadex(
		nullptr,
		0,
		ThreadProc,
		&hEvent,
		0,
		&threadID
	));

	for( ;; )
	{
		while( WaitForSingleObject(hEvent, 1) == WAIT_TIMEOUT ) {Sleep(1);}
		cout << "time elapsed" << endl;
		Sleep(1);

		if( !WaitForSingleObject(hThread, 1) /*WAIT_OBJECT_0*/ )
			break;
	}

	GetExitCodeThread(hThread, &threadExitCode);
	cout << threadExitCode << endl;

	CloseHandle(hEvent);
	CloseHandle(hThread);
	return 0;
}

unsigned __stdcall ThreadProc(void* param)
{
	HANDLE  hEvent = static_cast<HANDLE>(param);

	for( int i = 0; i < 10; ++i )
	{
		for( int j = 0; j < 1 << 31 - 1; ++j );
		PulseEvent(hEvent);
		Sleep(1);
	}

	_endthreadex(4711);
	return 0;
}

[FrEEzE2046]

Man sollte dazu sagen, dass ich heute seit Freitag krank bin. Anders kann ich mir diesen lächerlichen Fehler auch nicht erklären. In Zeile 47 muss es natürlich so aussehen:

HANDLE hEvent = *static_cast<HANDLE*>(param);

Jetzt funktioniert das auch.

[FrEEzE2046]

Ich hole diesen Thread nochmal vor. Wie kann ich denn eine Variable auf die gleichzeitig geschrieben werden könnte sicher lesen?

Ich nutze in meinem Code generell die Interlocked-Funktionen wenn möglich. Ich könnte natürlich mit InterlockedAnd oder InterlockedCompareExchange operieren, die mir jeweils den "alten" Wert zurückliefern, allerdings möchte ich weder austauschen noch konjugieren.

Kennt jemand eine andere Möglichkeit?

[hustbaer]

InterlockedCompareExchange(&v, 0, 0) geht schön, bzw. natürlich auch allgemein InterlockedCompareExchange(&v, X,

Mit MSVC (ab Version 8 aka 2005) + x86 reicht allerdings einfach ein Lesezugriff auf eine volatile Variable.

Bzw. glaube ich sogar ein normaler Lesezugriff wenn man ihn mit zwei _ReadWriteBarrier() Aufrufen einklammert.

EDIT:

MSDN schrieb:

Microsoft Specific

Objects declared as volatile are not used in certain optimizations because their values can change at any time. The system always reads the current value of a volatile object at the point it is requested, even if a previous instruction asked for a value from the same object. Also, the value of the object is written immediately on assignment.

Also, when optimizing, the compiler must maintain ordering among references to volatile objects as well as references to other global objects. In particular,
*
A write to a volatile object (volatile write) has Release semantics; a reference to a global or static object that occurs before a write to a volatile object in the instruction sequence will occur before that volatile write in the compiled binary.

A read of a volatile object (volatile read) has Acquire semantics; a reference to a global or static object that occurs after a read of volatile memory in the instruction sequence will occur after that volatile read in the compiled binary.
*
This allows volatile objects to be used for memory locks and releases in multithreaded applications.

Note

Although the processor will not reorder un-cacheable memory accesses, un-cacheable variables must be volatile to guarantee that the compiler will not change memory order.

End Microsoft Specific

[Patrick Star]

Ich begreif's nicht.

[Jonny boy]

Ich schon lang nicht mehr ...

[Andreas XXL]

Ich schlage diesen Beitrag für die FAQ vor!

(Wird zwar nicht so häufig gefragt, sind aber recht vernünftige Beiträge)

[xor]

Falls es um Geschwindigkeit geht, möchte ich noch die intrinsics des MSVC einbringen. So laufen die Operationen nicht als WinAPI Aufruf, sonder werden direkt als atomare CPU-Anweisung in den Code eingefügt. Und dass geht dann noch schneller als über die API.
Hab mir dafür mittels Makros die Funktionen umdefiniert ... hier ein Auszug:

#include "windows.h"

#if defined(CPP_MSVC05) && !defined(WINCE)
#  include "intrin.h"
#  define InterlockedExchange         _InterlockedExchange
#  define InterlockedCompareExchange  _InterlockedCompareExchange
#  define InterlockedIncrement        _InterlockedIncrement
#  define InterlockedDecrement        _InterlockedDecrement
#endif

lg XOR

[hustbaer]

defined(CPP_MSVC05) - huch?

ich mach immer defined(_MSC_VER) && (_MSC_VER >= ...)