Bug in Windows (10) gefunden?

hustbaer

Bonita.M schrieb:

Was für ein Unsinn.
Keiner entwickelt sowas und lässt das erst ein paar Jahre laufen.
Sowas guckt man sich im Source an und denkt das durch.

Ja, richtig.
Und man lässt es andere durchgucken.
Deswegen sollte man es auch dokumentieren so dass es für andere schnell verständlich ist. Der Teil fehlt bei deinem Code komplett. Eine Implementierung "reverse engineeren" zu müssen um draufzukommen wie sie funktionieren soll, um dann letztendlich prüfen zu können ob sie eben wirklich funktioniert, ist uninteressant.

Bonita.M

hustbaer schrieb:

Deswegen sollte man es auch dokumentieren so dass es für andere schnell verständlich ist. Der Teil fehlt bei deinem Code komplett. Eine Implementierung "reverse engineeren" zu müssen um draufzukommen wie sie funktionieren soll, um dann letztendlich prüfen zu können ob sie eben wirklich funktioniert, ist uninteressant.

Ich hab den Code einen amerikanischen Informatik-Professor geschickt von dem ich einen interessanten Artikel über die Implementation von POSIX Condition-Variablen für Win32 gelesen habe. Er hat den Code verstanden, hatte den Ansatz bisher noch nicht gesehen, und fand ihn interessant. Also: wenn man in der Thematik drinsteckt, also grundsätzlich weiß wie Synchronisationsprimitive arbeiten (das ist Wissen was man sich in der englischsprachigen Wikipedia anlesen kann), dann versteht man das auch. Das Thema ist einfach zu umfangreich, als dass ich die Basics in den Source reinschreiben wollte bzw. könnte; der Source wäre bestimmt viemal so groß. Kennt man die aber, versteht man auch was der Code macht. Das gleiche gilt für das kommende Transactional Memory (was es seit den Haswell-CPUs gibt und seit Syklake auch funktioniert). Wenn man damit arbeitet, dokumentiert man den Code nicht insofern als man die Basics im Umgang damit in den Code reinschreibt. Das ist eine völlig neue Denke die man erstmal kennenlernen muss.
BTW: Das Wartende die via WaitForMultipleObjects warten gegenüber denen die via WaitForSingleObject warten insofern nachrangig behandelt werden, dass sie ggf. nie drankommen, hielt der Professor auch für einen "conceptual flaw".

hustbaer

Ich habe nicht behauptet dass man den Code nicht verstehen kann. Ich habe behauptet dass es viel umständlicher ist wenn nix dabei steht.

Ich verstehe wie Synchronisationsprimitive funktionieren. Ich bin mir auch sicher dass ich deinen Code nachvollziehen könnte wenn ich mir die Zeit dafür nehme. Ich bin nur einfach nicht ausreichend motiviert Detektiv zu spielen. Damit meine ich nicht rauszubekommen wie Synchronisationsprimitive wie Events oder Semaphore funktionieren, sondern wie dein Code funktioniert. Bzw. genauer: funktionieren möchte. Wer sagt denn dass du alles bedacht hast, dass da keine Fehler drinnen sind? Und wenn Fehler drinnen sind, wie soll ein Fremder dann verstehen was du machen wolltest wenn du es nicht mit Kommentaren (oder einer externen Doku) beschreibst. Gerade wenn du schreibst dass es ein neuer Ansatz ist sollte klar sein dass man den nicht kennen wird.

z.B. was bedeutet "Owners()" und was "Waiters()". Dass du hier zwei getrennte Variablen in einem int64 ablegst, damit du sie gleichzeitig atomat ansprechen/updaten kannst, ist klar. Aber die Bedeutung der Variablen ist nicht klar.
Wenn ich mir z.B. das ansehe:

void CondVar::Enter()
{
    if( m_dwOwningThreadId == FastGetCurrentThreadId() )
    {
        m_dwRecursionCount++;
        return;
    }

    LONGLONG llOwnersAndWaiters = ::InterlockedIncrement64( &m_llOwnersAndWaiters  );

    assert(Owners( llOwnersAndWaiters ) > Waiters( llOwnersAndWaiters ));
...

Der "recursive lock" Teil ist klar.
Dann machst du mit dem InterlockedIncrement quasi "owners++". Das muss man sich aber schonmal raussuchen (gucken wie Owners() und Waiters() implementiert sind), weil man sonst nicht wissen kann was das InterlockedIncrement verändert. Das könnte schonmal als Kommentar dabeistehen.
Und dann das assert...

Wenn ich es mal umschreibe steht hier sinngemäss inetwa

/* snip recursive lock part */
    m_owners++;
    assert(m_owners > m_waiters);

Das ist erstmal total WTF???
Klar, wenn man dann etwas nachdenkt kommt man drauf dass es nicht mehr Threads geben darf die in Wait() hängen als die bereits erfolgreich Enter() aufgerufen haben. Das kann man aber auch dazuschreiben.

BTW:
Das...

m_dwOwningThreadId   = 0;
    dwSavedRecusionCount = m_dwRecursionCount;
    m_dwRecursionCount   = 0;

ist einfach nur ein Fehler.
Den Fall dass Wait mit dwSavedRecusionCount > 1 aufgerufen wird solltest du einfach nur mit assert() verbieten bzw. ggf. abort() aufrufen. Die Mutex einfach so aufzumachen ist mMn. einfach nur falsch. Grob falsch. Gefährlich falsch.

Dann der Loop hier

if( Owners( llOwnersAndWaiters ) > Waiters( llOwnersAndWaiters ) )
        for( ; !::SetEvent( m_xhEvtEnter.h ); assert(false) );

ist auch mehr als nur fragwürdig.
Wenn SetEvent fehlschlägt kannst du das hier nicht sinnvoll behandeln. Ein assert() ist OK, aber der folgende Retry ist es NICHT.

Ich bin sicher dass es da noch einiges gibt. Aber da du kein Interesse daran zu haben scheibst deinen Code gut lesbar und/oder verstehbar zu machen, hab ich auch kein grosses Interesse daran deinen Code durchzugucken.

Bonita.M

[code="cs"]

hustbaer schrieb:

BTW:
Das...

m_dwOwningThreadId   = 0;
    dwSavedRecusionCount = m_dwRecursionCount;
    m_dwRecursionCount   = 0;

ist einfach nur ein Fehler.

Nein, da ist kein Fehler. Du hast es nur nicht verstanden.
In dem Moment in dem ich das tue bin habe ich noch die Datenstruktur gelockt, und Owners ist > Waiters. Daher darf ich zu diesem Zeitpunkt noch m_dwOwningThread und m_dwRecursionCount verändern. Wenn ich dann aber Waiters inkrementiere, dann ist Owners möglicherweise sogar gleich Waiters, und niemand hat die Struktur gelockt, und alle die auf eine Signalisierung warten warten auch auf den Eintritt in den kritischen Abschnitt (deswegen auch die zwei Strukturen, das Semaphor für die Signalisierung, und das Event für den Eintritt in den kritischen Abschnitt).

hustbaer schrieb:

Wenn SetEvent fehlschlägt kannst du das hier nicht sinnvoll behandeln. Ein assert() ist OK, aber der folgende Retry ist es NICHT.

Das ist konzeptuell nicht anders möglich. Das darf *nie* fehlschlagen, sonst ist eine derart simple Implementation nicht möglich. CRITICAL_SECTION wirft undokumentiererweise wenn das fehlschlägt eine SEH-Exception, und das Programm terminiert. Das ist auch eine saubere Lösung.
Eigentlich kostet die mit SetEvent ausgelöste Wahl eines anderen Threads in die Runnable Liste sicher nur minimal Resourcen. Aber wenn die Resourcen so knapp sind, dass auch das fehlschlägt, dann läuft sicher auch nicht mehr der Debugger. Insofern ist das assert eigentlich Quatsch.

hustbaer

Bonita.M schrieb:

hustbaer schrieb:
BTW:
Das...
m_dwOwningThreadId   = 0;
    dwSavedRecusionCount = m_dwRecursionCount;
    m_dwRecursionCount   = 0;
ist einfach nur ein Fehler.
Nein, da ist kein Fehler. Du hast es nur nicht verstanden.

Lol
Nein.
Du hast es nicht verstanden.
Wait aufzurufen während man die Mutex doppelt gelockt hat ist ein Anwendungsfehler.
Deine Implementierung erlaubt das explizit, und gibt in dem Fall sämtliche rekursiven Locks frei. Und genau das ist der Fehler.

Nebenbei bemerkt: Dein Unit-Test testet die Wait-Funktion nicht effektiv. Wenn ich den laufen lasse wird Wait nie aufgerufen. Ich musste erst noch ein Sleep(1) vor dem ::cv.Enter(); in ThreadFunc einfügen damit das if (::dqtr.empty()) jemals true wird.

ps: Natürlich ist das dein Code, und du kannst definieren was du willst. Du kannst also auch definieren dass dein cv::Wait dieses verhalten hat. Dann ist es kein Fehler. Dann ist nur deine cv Klasse ein nutzloses Teil von dem man besser die Finger lassen sollte.

Bonita.M schrieb:

hustbaer schrieb:

Wenn SetEvent fehlschlägt kannst du das hier nicht sinnvoll behandeln. Ein assert() ist OK, aber der folgende Retry ist es NICHT.

Das ist konzeptuell nicht anders möglich. Das darf *nie* fehlschlagen, sonst ist eine derart simple Implementation nicht möglich. CRITICAL_SECTION wirft undokumentiererweise wenn das fehlschlägt eine SEH-Exception, und das Programm terminiert. Das ist auch eine saubere Lösung.
Eigentlich kostet die mit SetEvent ausgelöste Wahl eines anderen Threads in die Runnable Liste sicher nur minimal Resourcen. Aber wenn die Resourcen so knapp sind, dass auch das fehlschlägt, dann läuft sicher auch nicht mehr der Debugger. Insofern ist das assert eigentlich Quatsch.

Nein, das assert ist überhaupt nicht Quatsch.
Quatsch ist mMn. die Annahme dass der häufigste Fehlerfall bei SetEvent "zu wenig Resourcen" wäre. Ich wüsste nicht dass das überhaupt passieren kann. Wieso sollte SetEvent irgendwelche Resourcen benötigen?

Real mögliche Fehlerfälle sind aber 1) das übergebene Handle ist ungültig und 2) das übergebene Handle nat nicht die nötigen Berechtigungen.
Speziell (1) kann passieren wenn der Code der deine Mutex/CV Implementierung verwendet Mist baut (bzw. natürlich auch wenn dein Code Fehler enthält). Handle-Werte werden unter Windows recht schnell recycled. D.h. wenn der Anwendercode einen Fehler dieser Art enthält

HANDLE h = ...
CloseHandle(h);

g_something = new cv();

CloseHandle(h); // <-- Anwenderfehler

Dann kann es sein dass das 2. (falsche) CloseHandle deinen Event freigibt. In dem Fall unendlich zu loopen ist Quatsch. Da gehört ein assert+abort hin.

Bzw. natürlich genau so wenn der User das cv Objekt bereits zerstört hat.

Solche Fälle kann man nicht "korrekt" behandeln. Aber man kann helfen die Fehler schneller zu finden. Das assert macht also durchaus Sinn. Bzw. wenn du sicherstellen willst dass ein Programm welches so einen Fehler enthält nicht unkontrolliert weiterläuft, dann wie gesagt ein abort().

Ohne den Error-Code zu checken einfach zu loopen ist aber haarsträubender Quatsch.

dot

@hustbaer: du verschwendest hier deine Zeit...

Bonita.M

hustbaer schrieb:

Lol
Nein.
Du hast es nicht verstanden.
Wait aufzurufen während man die Mutex doppelt gelockt hat ist ein Anwendungsfehler.

Erstens ist kein doppeltes Locken möglich; es gibt immer nur einen Owner. Und zweitens hat der Thread bis zum InterlockedAdd den Besitz an dem Mutex, und kann bis dahin m_dwRecursionCount und m_dwOwningId ändern.

... Dann ist nur deine cv Klasse ein nutzloses Teil von dem man besser die Finger lassen sollte.

Der Code is 100% fehlerfrei. Ich habe ihn mit einer Queue die von einem Haupt-Thread 1.000 Worker-Threads füttert und einer Queue die von den 1.000 Worker-Threads wieder mit den Ergebnissen an den Hauptthread am laufen gehabt.

Bonita.M schrieb:

Nein, das assert ist überhaupt nicht Quatsch.
Quatsch ist mMn. die Annahme dass der häufigste Fehlerfall bei SetEvent "zu wenig Resourcen" wäre. Ich wüsste nicht dass das überhaupt passieren kann. Wieso sollte SetEvent irgendwelche Resourcen benötigen?

Du spekulierst hier wild rum. Du weißt aber nicht was SetEvent im Kernel alles lostritt.

Real mögliche Fehlerfälle sind aber 1) das übergebene Handle ist ungültig und 2) das übergebene Handle nat nicht die nötigen Berechtigungen.

Kommt bei mir nicht vor.

Martin Richter

Bonita.M schrieb:

Martin Richter schrieb:

Die Windows API hat 0 Zeilen...

Das was ich mit der Windows-API mache ist trivial.

Ich meine die Condition Variable, die in der Win-API integriert ist. Warum selber bauen?

Bonita.M

Martin Richter schrieb:

Ich meine die Condition Variable, die in der Win-API integriert ist. Warum selber bauen?

Weil es spaß macht und meine ein gutes Stück effizienter ist (z.B. macht die keine Spurious Wakeups).
Außerdem gibt es ja noch die Möglichkeit, dass jemand Code schreibt der vor Windows Vista bzw. Windows Server 2008 laufen muss; die haben nämlich keine Condition Variable.

hustbaer

Bonita.M schrieb:

hustbaer schrieb:

Lol
Nein.
Du hast es nicht verstanden.
Wait aufzurufen während man die Mutex doppelt gelockt hat ist ein Anwendungsfehler.

Erstens ist kein doppeltes Locken möglich; es gibt immer nur einen Owner.

Natürlich ist es das, dafür hast du ja extra m_dwRecursionCount eingebaut. Verstehst du wirklich nicht was ich schreibe oder willst du mich bloss verarschen?

Bonita.M schrieb:

Und zweitens hat der Thread bis zum InterlockedAdd den Besitz an dem Mutex, und kann bis dahin m_dwRecursionCount und m_dwOwningId ändern.

Ja, Mädchen. Ich versteh' dich schon. Du mich bloss nicht. Steig mal von deinem hohen Ross runter und lies Beiträge von anderen mal unter der Annahme dass sie vielleicht a) nicht blöd sind und b) Recht haben könnten.
Dashier geht mit deinem Code:

cs.Enter();
cs.Enter();
cs.Wait();
cs.Leave();
cs.Leave();

Während des Wait() kann die CV dabei von anderen Threads gelockt werden. UND DAS IST EIN FEHLER!

Bonita.M schrieb:

... Dann ist nur deine cv Klasse ein nutzloses Teil von dem man besser die Finger lassen sollte.

Der Code is 100% fehlerfrei. Ich habe ihn mit einer Queue die von einem Haupt-Thread 1.000 Worker-Threads füttert und einer Queue die von den 1.000 Worker-Threads wieder mit den Ergebnissen an den Hauptthread am laufen gehabt.

Kleine, du hast genau keinen Plan von der Praxis. So ein Test besagt nichts.

Bonita.M schrieb:

Nein, das assert ist überhaupt nicht Quatsch.
Quatsch ist mMn. die Annahme dass der häufigste Fehlerfall bei SetEvent "zu wenig Resourcen" wäre. Ich wüsste nicht dass das überhaupt passieren kann. Wieso sollte SetEvent irgendwelche Resourcen benötigen?

Du spekulierst hier wild rum. Du weißt aber nicht was SetEvent im Kernel alles lostritt.

Und? Du genau so wenig. Im Gegensatz zu mir hast du aber Code geschrieben der sehr starke Annahmen darüber macht was mögliche Fehlerfälle bei SetEvent sein könnten.

Bonita.M schrieb:

Real mögliche Fehlerfälle sind aber 1) das übergebene Handle ist ungültig und 2) das übergebene Handle nat nicht die nötigen Berechtigungen.

Kommt bei mir nicht vor.

Lol.
Du hast nichts verstanden.

Danke und tschüss.

Bonita.M

[quote]Natürlich ist es das, dafür hast du ja extra m_dwRecursionCount eingebaut. Verstehst du wirklich nicht was ich schreibe oder willst du mich bloss verarschen?[quote]

Wenn auf eine Rekursion getoffen wird wird aber nich gelockt, sondern nur der Rekursionszähler erhöht

Ja, Mädchen. Ich versteh' dich schon. Du mich bloss nicht. Steig mal von deinem hohen Ross runter und lies Beiträge von anderen mal unter der Annahme dass sie vielleicht a) nicht blöd sind und b) Recht haben könnten.
Dashier geht mit deinem Code:

Nein, Du verstehst meinen Code nicht.
Der US-Professor an den ich den geschickt habe hat den aber verstanden.

cs.Enter();
cs.Enter();
cs.Wait();
cs.Leave();
cs.Leave();
Während des Wait() kann die CV dabei von anderen Threads gelockt werden. UND DAS IST EIN FEHLER!

Nein, kann nicht, weil beim wait Owners > waiters ist. Ist Owner - Waiters == 1 gibt es genau einen Thread der das Lock gelockt hat, ist es > 1, gibt es einen Thread der das Lock gelockt hat und zusätzlich Owners - Waiters Threads die auf das Entriegeln warten.

[quote="Bonita.M"]Und? Du genau so wenig. Im Gegensatz zu mir hast du aber Code geschrieben der sehr starke Annahmen darüber macht was mögliche Fehlerfälle bei SetEvent sein könnten.[quote]

SetEvent gibt einen Fehlerode zurück, und ich gehe damit richtig um.

Du hast nichts verstanden.

Du hast nichts verstanden. Meintest Du bist in der Lage Locking-Primitive zu verstehen, kannst es aber offensichtlich nicht.
Wenn keiner den Speicherblock zerschießt in dem mein Lock liegt, dann haben wir es mit einem gültigen Handle zu tun - dann kann SetEvent nur noch aus anderen Gründen fehlschlagen, mglw. weil eben nicht genug Resourcen da sind um einen Thead der nun runnalble wird auf die Runnable-Liste zu packen. Für den Fehlerfall, dass jemand den Speicherblock zerschießt in dem das Handle liegt kann ich nichts.

Bonita.M

Natürlich ist es das, dafür hast du ja extra m_dwRecursionCount eingebaut. Verstehst du wirklich nicht was ich schreibe oder willst du mich bloss verarschen?

Wenn auf eine Rekursion getoffen wird wird aber nich gelockt, sondern nur der Rekursionszähler erhöht

Ja, Mädchen. Ich versteh' dich schon. Du mich bloss nicht. Steig mal von deinem hohen Ross runter und lies Beiträge von anderen mal unter der Annahme dass sie vielleicht a) nicht blöd sind und b) Recht haben könnten.
Dashier geht mit deinem Code:

Nein, Du verstehst meinen Code nicht.
Der US-Professor an den ich den geschickt habe hat den aber verstanden.

cs.Enter();
cs.Enter();
cs.Wait();
cs.Leave();
cs.Leave();
Während des Wait() kann die CV dabei von anderen Threads gelockt werden. UND DAS IST EIN FEHLER!

Nein, kann nicht, weil beim wait Owners > waiters ist. Ist Owner - Waiters == 1 gibt es genau einen Thread der das Lock gelockt hat, ist es > 1, gibt es einen Thread der das Lock gelockt hat und zusätzlich Owners - Waiters - 1 Threads die auf das Entriegeln warten. Zugegebenermaßen ist der Begriff Owner hier bissl verkehrt, da nur einer von Owner - Waiters tatsächlich das Lock gelockt hält.

Und? Du genau so wenig. Im Gegensatz zu mir hast du aber Code geschrieben der sehr starke Annahmen darüber macht was mögliche Fehlerfälle bei SetEvent sein könnten.

SetEvent gibt einen Fehlerode zurück, und ich gehe damit richtig um.

Du hast nichts verstanden.

Du hast nichts verstanden. Meintest Du bist in der Lage Locking-Primitive zu verstehen, kannst es aber offensichtlich nicht.
Wenn keiner den Speicherblock zerschießt in dem mein Lock liegt, dann haben wir es mit einem gültigen Handle zu tun - dann kann SetEvent nur noch aus anderen Gründen fehlschlagen, mglw. weil eben nicht genug Resourcen da sind um einen Thead der nun runnable wird auf die Runnable-Liste zu packen. Für den Fehlerfall, dass jemand den Speicherblock zerschießt in dem das Handle liegt kann ich nichts.

Bonita.M

löschen

Mechanics

Bonita.M schrieb:

Der US-Professor an den ich den geschickt habe hat den aber verstanden.

Das ist nicht beeindruckend. Ich hab den Thread nur überflogen und hab keine Ahnung, von dem eigentlichen Problem, aber ich möchte dich darauf hinweisen, dass Professoren normalerweise keinen besonders guten Ruf haben, bis auf wenige Ausnahmen. Außerdem können wir aus dem Kontext gerissen auch nicht wissen, ob er sich in das Problem überhaupt reingedacht hat oder ob ers nur überflogen hat. Dass er die Idee interessant fand, bedeutet nicht, dass er auch ausschließen kann, dass da Fehler oder potentielle Probleme sind.

hustbaer

Du hast immer noch nicht verstanden was ich dir sagen will.
Vielleicht hilft ein Beispiel. Klasse cv wie von dir verlinkt, folgender Test-Code:

#include <cstdio>
#include <cstdlib>
#include <iostream>

DWORD WINAPI ThreadFunc(LPVOID lpvThreadParam);

CondVar cv;

int main()
{
	std::cout << "Locking...\n";
	::cv.Enter();
	std::cout << "Locking again (recursive)...\n";
	::cv.Enter();

	std::cout << "Starting thread...\n";

	HANDLE th = CreateThread(NULL, 0, ThreadFunc, NULL, 0, NULL);

	std::cout << "Waiting...\n";
	::cv.Wait();
	std::cout << "We have been signaled.\n";

	std::cout << "Unlocking...\n";
	::cv.Leave();
	::cv.Leave();

	std::cout << "Bye.\n";

	::WaitForSingleObject(th, INFINITE);
	::CloseHandle(th);
}

DWORD WINAPI ThreadFunc(LPVOID lpvThreadParam)
{
	::cv.Enter();
	std::cout << "WE SHOULD NEVER GET HERE!\n";
	::cv.Release();
	::cv.Leave();

	return 0;
}

Das geht, darf aber nicht gehen.
Klar jetzt?

SetEvent gibt einen Fehlerode zurück, und ich gehe damit richtig um.

Nein, tust du nicht. Ich habe dir auch beschrieben warum. Dass dir das egal ist, bzw. es dir wichtiger zu sein scheint dass du keinen Fehler zugeben musst, OK. Kann mir im Prinzip egal sein. Schreib halt weiter schlechten Code.

Bonita.M

Ich hab dich schon verstanden. Im Gegensatz zum vor-vorangegangen Posting hast Du dich mal ausnahmsweise nicht mehrdeutig ausgedrückt.
Natürlich darf das gehen, mein Lock ist ja rekursiv. Wieso soll man auf eine doppelt gelockte CV nicht warten können?
Die inneren Lock- und Leave-Aufrufe machen mit dem Lock garnichts außer den Rekursionszähler zu erhöhen und wieder zu erniedrigen. Wait speichert diesen zwischen, und wenn es ein Signal emfpangen hat, restauriert es den wieder. Gelockt oder entlockt wird also durch die inneren Aufrufe nicht. Das ist absolut kein Bug, dass das geht, sondern so beabsichtigt.

Wenn mein Unit-Test jetzt so aussähe ...

DWORD WINAPI ThreadFunc( LPVOID lpvThreadParam );

struct ThreadResult
{
	DWORD dwThreadId;
	DWORD dwCounter;
};

CondVar            cv;
list<ThreadResult> lstTR;
bool volatile      fStop = false;

int main()
{
	int const              NTHREADS = 16;
	HANDLE                 ahThreads[NTHREADS];
	int                    i;
	std::map<DWORD, DWORD> mTRs;

	for( i = 0; i < NTHREADS; i++ )
		ahThreads[i] = CreateThread( NULL, 0, ThreadFunc, NULL, 0, NULL );

	for( i = 0; i < 10000; i++ )
	{
		ThreadResult tr;

		::cv.Enter();
		::cv.Enter();

		if( ::lstTR.empty() )
			::cv.Wait();

		tr = ::lstTR.front();
		::lstTR.pop_front();
		::cv.Leave();
		::cv.Leave();

		printf( "Thread: %08X - Number: %d\n", (unsigned)tr.dwThreadId, (unsigned)tr.dwCounter );

		if( mTRs.find( tr.dwThreadId ) == mTRs.end() )
			assert(tr.dwCounter == 0),
			mTRs[tr.dwThreadId] = tr.dwCounter;
		else
			assert((mTRs[tr.dwThreadId] + 1) == tr.dwCounter),
			mTRs[tr.dwThreadId] = tr.dwCounter;
	}

	for( ::fStop = true, i = 0; i < NTHREADS; i++ )
		WaitForSingleObject( ahThreads[i], INFINITE );

	return 0;
}

DWORD WINAPI ThreadFunc( LPVOID lpvThreadParam )
{
	ThreadResult tr;

	tr.dwThreadId = GetCurrentThreadId();
	tr.dwCounter  = 0;

	for( ; !::fStop; tr.dwCounter++ )
	{
		::cv.Enter();
		::cv.Enter();
		::lstTR.push_back( tr );
		::cv.Release();
		::cv.Leave();
		::cv.Leave();
		Sleep( 10 );
	}

	return 0;
}

...dann ist das von der Funktionsweise so als würden die inneren Locks und Leaves nicht exisitieren. Der Code läuft genau so wie gewollt durch.

Und die Endlosschleife bei SetEvent ist kein schlecher Code - das geht nicht anders weil die CV so simpel gestrickt ist. Das assert ist eigentlich überflüssig.

hustbaer

Achherrjeh, Bonita, ich weiss was rekursive Mutexen sind und wie sie funktionieren. Ich weiss auch was du dir dabei gedacht hast. Ich versuche dir nur zu erklären dass es trotzdem unsinnig ist. Es maskiert gefährliche Anwendungsfehler.

Wenn du es mir nicht glaubst, dann guck doch mal ob du eine Mutex/CV Implementierung findest die so arbeitet. Ich kenne keine. Und das hat schon seinen Grund.

Bonita.M schrieb:

Und die Endlosschleife bei SetEvent ist kein schlecher Code - das geht nicht anders weil die CV so simpel gestrickt ist. Das assert ist eigentlich überflüssig.

mimimi
Ne, ich erklärs dir jetzt nicht nochmal.

Bonita.M

hustbaer schrieb:

Achherrjeh, Bonita, ich weiss was rekursive Mutexen sind und wie sie funktionieren. Ich weiss auch was du dir dabei gedacht hast. Ich versuche dir nur zu erklären dass es trotzdem unsinnig ist. Es maskiert gefährliche Anwendungsfehler.

Das doppelte Locken und darin das Wait mag in der Praxis kaum vorkommen, aber es ist kein Anwendungsfehler. Ich finde es nicht falsch diesen seltenen Fall mit zu unterstützen.
Hättest Du dich direkt von Anfang an vollständig und nicht mehrdeutig ausgedrückt, dann hätte ich deine seltsamen Bedenken gleich verstanden. Das hast Du aber nicht.

hustbaer

Es ist sehr oft ein Anwendungsfehler. Und wenn es keiner ist, dann ist das rekursive Locken vermeidbar. Es gibt also keinen Grund Wait einen rekursiven Unlock machen zu lassen. Es gäbe dagegen gute Gründe in Wait ein assert() einzubauen um abzusichern dass der Fall eben nicht auftritt.

Aber du weisst ja alles besser.

hustbaer

Bonita.M schrieb:

Hättest Du dich direkt von Anfang an vollständig und nicht mehrdeutig ausgedrückt, dann hätte ich deine seltsamen Bedenken gleich verstanden. Das hast Du aber nicht.

Ja, klar, liegt bestimmt an mir.
Muss so sein. Du machst ja keine Fehler. Und verstehst auch alles.