Bug in Windows (10) gefunden?

[Bonita.M]

Ich habe eine Lösung gefunden.

So sieht mein neues main() aus:

int main()
{
	int const NTHREADS = 2;
	HANDLE    ahWait[2];

  	ahWait[0] = ::hEvt  = CreateEvent( NULL, FALSE, TRUE, NULL );
	ahWait[1] = ::hSema = CreateSemaphore( NULL, 0, 1, NULL );
	fReleased = false;

	for( int i = 0; i < NTHREADS; i++ )
		CreateThread( NULL, 0, LockAndReleaseThread, NULL, 0, NULL );

	for( ; ; )
		WaitForSingleObject( ::hSema, INFINITE ),
		WaitForSingleObject( ::hEvt, INFINITE),
//		WaitForMultipleObjects( 2, ahWait, TRUE, INFINITE ),
		printf( "main thread is holding lock and received signal\n" ),
		::fReleased = false,
		SetEvent( hEvt );

    return 0;
}

Is nen Stück ineffizienter, aber so funktioniert's.
Würde ich übrigens in umgekehrter Reihenfolge auf das Semaphor und das Event warten, dann gäb es einen Deadlock.

[Martin Richter]

Jupp. Ich kann das Verhalten sowohl mit Autoreset als auch mit einem Mutex nachvollziehen.
WaitForMultileObjects scheint "langsamer" zu sein, als WaitForSingleObject...

Aber ganz verstehe ich Dein Ziel nicht. Ein AutoReset Event ist kein Lock! Denn es bekommt ihn wohl einer genau signalisiert, aber ein zweiter kann genauso schnell kurz danach diesen Event signalisiert bekommen.
Genau genommen ist die Nutzung des Codes ab if (!::Released) nicht sicher...

Szenario:

1. Thread A läuft auf WaitForSingleObject, der Code wird ausgeführt, das Event zurückgesetzt und neu gesetzt.
2. Thread A läuft auf !::fReleased und führt ReleaseSemaphore aus.
3. Jetzt kommt (vielleicht) der MainThread an die Reihe und der Semaphore wird wieder gesetzt, das Flag fReleased wird auf false gesetzt.
4. Nun kommt Thread A wieder an die Reihe kommt aus Release Semaphore zurück und setzt fReleased auf true!

Ein AutoReset event ist ein Lock. Das Szenario kann ich aktuell natürlich nicht erzeugen, durch den PingPong Effekt.

Also habe ich Deinen Code umgeschrieben und einen Mutex verwendet, der genau diesen Codeteil um fReleased schützt. Das Verhalten ersacheint mir jetzt auch korrekt.

#include <windows.h>
#include <unordered_map>
#include <stdio.h> 

HANDLE        hSema, hMtx;
bool volatile fReleased = false;

DWORD WINAPI LockAndReleaseThread(LPVOID lpvThreadParam);

int main()
{
	int const NTHREADS = 4;
	HANDLE    ahWait[2];

	ahWait[0] = ::hSema = CreateSemaphore(NULL, 0, 1, NULL);
	ahWait[1] = ::hMtx = CreateMutex(NULL, FALSE, NULL);

	for (int i = 0; i < NTHREADS; i++)
		HANDLE hThread = CreateThread(NULL, 0, LockAndReleaseThread, NULL, 0, NULL);

	for (;;)
	{
		WaitForMultipleObjects(2, ahWait, TRUE, INFINITE);
		if (::fReleased)
		{
			printf("main thread is holding lock and received signal\n");
			::fReleased = false;
		}
		::ReleaseMutex(hMtx);
	}

	return 0;
}

char GetID();

DWORD WINAPI LockAndReleaseThread(LPVOID lpvThreadParam)
{
	for (;;)
	{
		WaitForSingleObject(hMtx, INFINITE);
		printf("spawned thread with id %c is holding lock\n", (char)GetID());

		if (!::fReleased)
			ReleaseSemaphore(::hSema, 1, NULL),
			::fReleased = true;

		::ReleaseMutex(hMtx);

		Sleep(1000);
	}

	return 0;
}

char GetID()
{
	static std::unordered_map<DWORD, char> mapTIDsToIDs;
	static char                            nextId = 'A';
	DWORD                                  dwThreadId;

	if (mapTIDsToIDs.find(dwThreadId = GetCurrentThreadId()) == mapTIDsToIDs.end())
		return mapTIDsToIDs[dwThreadId] = nextId++;
	else
		return mapTIDsToIDs[dwThreadId];
}

[Martin Richter]

Upps da warst Du schneller.

Dein Code ist nicht sicher in Bezug auf fReleased!

[dot]

Nur mal rein prinzipiell: Wieso nicht einfach std::condition_variable verwenden!? Oder die in Windows eingebauten Condition Variables!?

[Bonita.M]

Martin Richter schrieb:

Dein Code ist nicht sicher in Bezug auf fReleased!

Doch, ist er. Selbst wenn er nicht volatile wär ging das gut weil WaitForSingle/Multiple/Object(s) mindestens acquire- und SetEvent mindestens release-Verhalten hat.
Das ist schon allein deswegen so weil der Compiler nicht wissen kann ob die genannten beiden Funktionen diese Variable verändern (tun sie nicht, aber das kann der Compiler nicht wissen), und deswegen muss er auf Architekturen wie ARM entsprechende Fences setzen.

[Martin Richter]

Bonita.M schrieb:

Martin Richter schrieb:

Dein Code ist nicht sicher in Bezug auf fReleased!

Doch, ist er WaiForXxxObjects hat acuire- und release-Verhalten.

Nicht in Bezug auf WaitFor...

Schau Dir die Variable fRelease an.

Diese wird verändert nachdem Der Event getriggert wurde. Aber nichts spricht dagegen, das zwei Threads in kürzester Zeit den Event bekommen und im Falle des Mainthreads einmal das flag auf false und im anderen Fall auf true setzen.

Folgende Zeitfolge für Thread A und Mainthread M

A: WaitForMultipleObjects( 1, &::hEvt, TRUE, INFINITE );
A: if (!::fReleased)
A: ReleaseSemaphore(::hSema, 1, NULL),
M: WaitForMultipleObjects(2, ahWait, TRUE, INFINITE);
M: if (::fReleased)
A: ::fReleased = true;

...

Das Event is kein Lockmechanismus. Kurz hintereinander können sehr wohl mehrere Threads diesen Wait überlaufen.

Aber ohne weiteres Wissen was die Worker machen, kann man natürlich nicht sagen, ob das gefährlich ist.

[Bonita.M]

löschen!

[Bonita.M]

Martin Richter schrieb:

Schau Dir die Variable fRelease an.

Nene, der Code ist einwandfrei.
fRelase wird erstmal nur angefasst wenn man den kritischen Abschnitt mit WairForXxxObject(s) betreten hat. Und gesetzt wird sie von LockAndReleaseThread auch nur wenn er in dem kritischen Abschnitt ist und ::fReleased false ist. Und zurückgesetzt wird er nur vom Hauptthread wenn das Semaphor gesetzt wurde; dan ist nämlich auch ::fReleased true.

Martin Richter schrieb:

A: WaitForMultipleObjects( 1, &::hEvt, TRUE, INFINITE );
A: if (!::fReleased)
A: ReleaseSemaphore(::hSema, 1, NULL),

Wieso soll als nächstes M drankommen? Das Event ist doch nicht gesetzt!

Martin Richter schrieb:

M: WaitForMultipleObjects(2, ahWait, TRUE, INFINITE);
M: if (::fReleased)
A: ::fReleased = true;

[Martin Richter]

Nein! fRelease ist true NACHDEM ReleaseSemaphore ausgeführt wurde.
Dazwischen kann alles passieren!

Nachtrag: Du änderst gerade den Thread auf den ich mich beziehe!

[Bonita.M]

Martin Richter schrieb:

Nein! fRelease ist true NACHDEM ReleaseSemaphore ausgeführt wurde.

Mann, Mann, Mann, Du hast echt Tomaten auf den Augen.
Das ist überhauptkein Problem, denn der Hauptthread schafft es erst an ::fReleased heran wenn das Event am Ende von LockAndReleaseThread wieder freigegeben wird.
Schau dir den Code vor dem nächsten Posting nochmal sorgfältig an!

[Martin Richter]

Ein normaler und netter Ton wäre OK.

Deine Funktion heißt LOCK! Es ist kein Lock UND ich habe geschrieben, dass ohne weiteres Wissen, was in diesem Block noch passiert ich mir kein abschließendes Urteil erlauben kann. Aber es ist KEIN LOCK!

Ansonsten ist für mich die Diskussion beendet. Danke, weitere Kommentare und Antworten sind nicht mehr notwendig.

[Bonita.M]

Martin Richter schrieb:

Deine Funktion heißt LOCK! Es ist kein Lock ...

Man kann mit einem Autoreset-Event ein Lock realisieren. Das funktioniert dann genauso wie eine CRITICAL_SECTION, nur nicht so effizient (die CRITICIAL_SECTION lockt für den Fall das beide nicht überschneidend locken wollen im Userland durch eine atomare Operation).
Intern benutzt CRITICAL_SECTION auch ein Event für den Fall, dass überschneidend auf den kritischen Abschnitt zugegriffen wird.
Daher ist die Bezeichnung *Lock*AndReleaseThread hier voll angebracht.

Deinen MVP hast Du sicher nicht durch Erkenntnisse im Multithreading bekommen.

[hustbaer]

@Bonita.M

Sieht so aus, als würde Windows die Threads die nur auf ein Ereignis warten bevorzugen, und die die auf mehrere warten kommen nicht zum Zuge. Ich finde das ist ein Bug.

Mehr als ein herzhaftes LOL hat das eigentlich nicht verdient.

[Bonita.M]

hustbaer schrieb:

Mehr als ein herzhaftes LOL hat das eigentlich nicht verdient.

Was intelligentes hast Du wohl nicht zu sagen.

[dot]

Bonita.M schrieb:

Man kann mit einem Autoreset-Event ein Lock realisieren.

ja

Bonita.M schrieb:

Das funktioniert dann genauso wie eine CRITICAL_SECTION, [...]

nö

Bonita.M schrieb:

Intern benutzt CRITICAL_SECTION auch ein Event für den Fall, dass überschneidend auf den kritischen Abschnitt zugegriffen wird.

nö

[Bonita.M]

dot schrieb:

Bonita.M schrieb:

Man kann mit einem Autoreset-Event ein Lock realisieren.

ja

Bonita.M schrieb:

Das funktioniert dann genauso wie eine CRITICAL_SECTION, [...]

nö

Bonita.M schrieb:

Intern benutzt CRITICAL_SECTION auch ein Event für den Fall, dass überschneidend auf den kritischen Abschnitt zugegriffen wird.

nö

Was bist Du denn für ein Schwachkopf?
Wenn ich in einem Thread auf einem Autoreset-Event WairForSingleObject( hEvt, INFINTE ) sage und später wieder SetEvent( hEvt ), dann ist das die gleiche Funktion wie bei einer CRITICAL_SECTION, nur nicht so effizient.
Und guck doch mal in den Header in dem CRITICAL_SECTION definiert ist. Da siehst du dann ein HANDLE für ein Event. Ich hab EnterCriticalSection und LeaveCriticalSection mal disassembliert, und MS macht da genau das was andere auch machen, nämlich mit eeinm Lock-Flag arbeiten das atomar geändeert wird, und für den Contention-Fall wird auf die Kernel-Betriebsmittel in beschriebener Art und Weise zugegriffen.
Ich habe in meinem ersten Posting die Implementation einer effizienten Condition-Variable verlinkt. Darin steckt auch ein Mutex bzw eine Critical Section. Und da mache ich genau das selbe wie EnterCriticalSection und LeaveCriticalSection, nur einen Tick effizienter.
Selten so einen ignoranten Dummkopf gelesen wie dich.

[EOP]

Ich halte es da mit Mr.Flounder:

The Best Synchronization Is No Synchronization

http://www.flounder.com/no_synchronization.htm
Ansonsten geht mir die Diskussion etwas auf den Keks.

[dot]

Bonita.M schrieb:

Was bist Du denn für ein Schwachkopf?

lol, na wenn dir eh alles klar ist, wieso verschwendest du hier dann deine Zeit damit, so Schwachköpfe wie uns Dinge zu fragen, auf die eh nur du allein die Antwort kennst?

Bonita.M schrieb:

Und guck doch mal in den Header in dem CRITICAL_SECTION definiert ist. Da siehst du dann ein HANDLE für ein Event. Ich hab EnterCriticalSection und LeaveCriticalSection mal disassembliert [...]

Meine Windows Header sind wohl leider völlig veraltet, vielleicht kannst du mir den aktuellen mal schicken? Und was für einen tollen Disassembler du da hast! Meiner zeigt mir leider die ganzen Event Syscalls irgendwie nicht an, vermutlich kann der in Sections, die in Paralleluniversen liegen, nicht rein...

Bonita.M schrieb:

Ich habe in meinem ersten Posting die Implementation einer effizienten Condition-Variable verlinkt.

Ja, die ist ja offenbar so wahnsinnig effizient, dass sie schon gar nicht mehr korrekt ist, manch ein Thread flutscht da ja durch als wär er nie drangekommen; nur die Performance is dafür halt nicht so toll und Systemressourcen verschwendet sie auch, aber darum geht's ja nicht; ich wünschte, so Schwachköpfe wie ich könnten sich auch mal sowas geniales einfallen lassen...

Das ist jedenfalls definitiv ein Bug in Windows 10 (damals auf XP war das ja alles noch besser); die Windows Kernel Entwickler sind halt auch nur unwesentlich weniger schwachköpfig als wir alle hier. Aber es kann halt nicht jeder soviel von paralleler Programmierung verstehen wie du, wir machen das ja nur beruflich...

[EOP]

Kannst dich ja immernoch als "Conchita Wurscht" anmelden. Du Held du.
Ja, jetzt werde ich etwas stänkerig, nachdem ich mir das Alles angesehen habe.
Wenn ich admin wäre würde sofort deine komplette IP Class B Range rausfliegen.

[hustbaer]

Bonita.M schrieb:

hustbaer schrieb:

Mehr als ein herzhaftes LOL hat das eigentlich nicht verdient.

Was intelligentes hast Du wohl nicht zu sagen.

Nicht jemandem der so austeilt wie du. Ausserdem will ich ja nicht dein Weltbild erschüttern.

Wenn du meinst eine effiziente CV Implementierung zu haben, dann lass sehen. Als Klasse mit den üblichen Funktionen, so dass man sinnvoll darüber argumentieren kann. Und wenns geht vielleicht ohne Komma-Operator oder sonstigen unüblichen Konstrukten.

Und was deine Behauptung dass das von dir beobachtete Verhalten ein Fehler sein soll angeht... naja die ist erstmal wirklich einfach nur lachhaft. Da müsste schon ein Link zu ner Doku dabei sein wo bezüglich "Fairness" beim Aufwecken der wartenden Threads irgendwas garantiert wird. Oder wenigstens nicht-garantiertes aber aktuell implementiertes Verhalten beschrieben wird.
Hast du sowas, dann lass sehen.

Ich kenne nur das Gegenteil. z.B. wo Microsoft unmisverständlich dokumentiert dass Dinge wie CRITICAL_SECTIONs nicht "fair" sind. Siehe
https://msdn.microsoft.com/en-us/library/windows/desktop/ms682608(v=vs.85).aspx
Der Satz der mit "There is no guarantee" anfängt.
Hat jetzt mit WaitForSingleObject/WaitForMultipleObjects nix zu tun, aber wie gesagt nur ein Beispiel - ich bin ja hier nicht in der Bringschuld.