DirectPlay: ServerMessageHandler-Hook thread-safe machen

ceplusplus*loggedoff

Hey!

Ich habe den "ServerMessageHandler" eines DirectPlay-Servers gehooked. Dies muss sein, da ich einen AntiCheat machen möchte.

( http://doc.51windows.net/Directx9_SDK/play/ref/callbacks/pfndpnmessagehandler.htm )

Nun, da dachte ich mir, mache ich es doch einfach so:

HRESULT CALLBACK hook_ServerMessageHandler(PVOID pvUserContext, DWORD dwMessageType, PVOID pMessage)
{
	EnterCriticalSection(&cs);

	// AntiCheat

	LeaveCriticalSection(&cs);

	return orig_ServerMessageHandler(pvUserContext, dwMessageType, pMessage);
}

Doch leider haut das nicht so recht hin:
Manchmal, wenn mehr als ein Spieler im Server ist, hängt sich der Server nach spätestens ein paar Minuten plötzlich auf, die Spieler werden gekickt wegen "AntiCheat answer timeout or bad connection" und man kann den Server anschließend nicht mehr betreten.

Da dachte ich dann an einen deadlock, aber warum sollte das passieren?

(Laut Doku... pro Spieler maximal ein Call (thread), wenn der fertig, dann erst der nächste vom selben Spieler)
Spieler 1: Try, Lock
Spieler 2: Try, Wait
Spieler 1: Process
Spieler 2: Try, Wait
Spieler 1: Leave
Spieler 2: Try, Lock
Spieler 1: Try, Wait
Spieler 2: Process
Spieler 1: Try, Wait
Spieler 2: Leave
usw...

Oder könnte es sein, dass ein thread plötzlich ganz ausgeschlossen wird? Also dass sobald der Spieler 1-Handler fertig ist, er gleich wieder lockt? Aber kann doch eigentlich nicht sein...

Oder was wäre, falls der Server so programmiert ist, dass Spieler 1-Handler auf irgendetwas von Spieler 2-Handler wartet oder so? Und da Spieler 2-Handler blockiert, weil Spieler 1-Handler dran ist, geht nichts mehr... Was könnte ich dann tun? Aber das kann doch irgendwie auch nicht sein, denn der originale ServerMessageHandler wird doch auch CriticalSection verwenden?

Woran könnte es noch liegen? Ohne CriticalSection funktioniert natürlich alles, aber dann kann ich keinen AntiCheat machen.

Ist mir sehr wichtig ...
Danke!
MfG

rapso

eventuell endlosloop im anti-cheat code
oder der anticheatcode ist so langsam, dass er nicht alle threads seriell abarbeiten kann.

einfacher test. lass die critical section drinnen aber kommentier den anti-cheatcode raus

wenn es dann immer noch laeuft, baue ein sleep ein, z.b. von 10 oder 100 und schaue ob dann ob das problem auftritt.

ceplusplus*loggedoff

Hey!
Ne, ich hab den Code so wie oben ausprobiert. Nur ne CriticalSection, sonst nix. Server hat sich bei >1 Spielern trotzdem aufgehangen... Das mit dem Sleep werde ich beizeiten probieren, danke!

Nun, aber ich frage mich ob ich in meinem Fall überhaupt thread-safety benötige. Denn ich habe zwar ein globales std::map Objekt, jedoch arbeite ich pro Spielermessage mit unterschiedlichen Einträgen, also nur dem aktuellen Spieler. Die Doku meint: "Callback messages from the same player are serialized".

Was ich genau mache:

Eine globale std::map.
Wird ein Spieler erstellt, füge ich ihn der map hinzu.
Wird ein Spieler zerstört, lösche ich ihn aus der map.
Hat ein Spieler eine bestimmte Anzahl an Nachrichten gesendet, lösche ich ihn ebenfalls aus der map (Analyse beendet sozusagen).
Also pro Spieler werden vorerst mal nur die ersten paar Pakete nach einem connect validiert (Da kann man nämlich viel Blödsinn senden und den Server somit crashen...).

Am besten mal bisschen Code:

struct Check
{
	int msgCount;
	bool danger;
	bool ackSent;

	Check() : msgCount(0), danger(false), ackSent(false)
	{
		;
	}
};

map<DPNID, Check> checkList;

HRESULT CALLBACK hook_ServerMessageHandler(PVOID pvUserContext, DWORD dwMessageType, PVOID pMessage)
{
	// EnterCriticalSection(&cs);

	if(dwMessageType == DPN_MSGID_CREATE_PLAYER)
	{
		PDPNMSG_CREATE_PLAYER pMsgCP = (PDPNMSG_CREATE_PLAYER)pMessage;
		checkList.insert(pair<DPNID, Check>(pMsgCP->dpnidPlayer, Check()));
	}

	if(dwMessageType == DPN_MSGID_DESTROY_PLAYER)
	{
		PDPNMSG_DESTROY_PLAYER pMsgDP = (PDPNMSG_DESTROY_PLAYER)pMessage;

		map<DPNID, Check>::iterator it = checkList.find(pMsgDP->dpnidPlayer);
		if(it != checkList.end())
			checkList.erase(it);
	}

	if(dwMessageType == DPN_MSGID_RECEIVE)
	{
		PDPNMSG_RECEIVE pMsgRecv = (PDPNMSG_RECEIVE)pMessage;

		// VALIDATION ROUTINE
		// Wenn aktuelle Nachricht von einem Spieler in der checkListe ist...
		map<DPNID, Check>::iterator it = checkList.find(pMsgRecv->dpnidSender);
		if(it != checkList.end())
		{
			++it->second.msgCount;

			switch(it->second.msgCount)
			{
			case 1:
				// Validieren... eventuell blocken
				break;
			// ...
			case 7:
				// Genügend Nachrichten analysiert, Spieler nicht weiter analysieren
				checkList.erase(it);
				break;
			}
		}
	}

	// LeaveCriticalSection(&cs);

	return orig_ServerMessageHandler(pvUserContext, dwMessageType, pMessage);
}

Ich denke, hier benötige ich garkeine thread-safety, denn in einem callback wird immer nur der aktuelle Spieler behandelt (Spieler1-Callback: Nur Spieler1-Eintrag der map, Spieler2-Callback: Nur Spieler2-Eintrag der map und sowieso immer nur ein callback pro Spieler).

Aber std::find() und std::erase() werden halt trotzdem in mehreren callbacks aufgerufen. Gibt das keine Probleme?
Meine damit zB:
Spieler1-Callback beginnt mit find()... threadwechsel...
Spieler2-Callback beginnt ebenfalls mit find()...

Danke!
MfG

rapso

ceplusplus@loggedoff schrieb:

Hey!
Ne, ich hab den Code so wie oben ausprobiert. Nur ne CriticalSection, sonst nix. Server hat sich bei >1 Spielern trotzdem aufgehangen... Das mit dem Sleep werde ich beizeiten probieren, danke!

und wenn du einfach nur die critical section auskommentierst, laeuft alles ohne probleme?
eigentlich duerfte enter critical section garkeine probleme machen.

Ich denke, hier benötige ich garkeine thread-safety, denn in einem callback wird immer nur der aktuelle Spieler behandelt (Spieler1-Callback: Nur Spieler1-Eintrag der map, Spieler2-Callback: Nur Spieler2-Eintrag der map und sowieso immer nur ein callback pro Spieler).

Aber std::find() und std::erase() werden halt trotzdem in mehreren callbacks aufgerufen. Gibt das keine Probleme?
Meine damit zB:
Spieler1-Callback beginnt mit find()... threadwechsel...
Spieler2-Callback beginnt ebenfalls mit find()...

Danke!
MfG

es gibt probleme sobald die map modifiziert wird. nicht nur wenn gerade zwei funktionen gleichzeitig aufgerufen werden, auch sonst kann der iterator invalidiert werden.

ceplusplus*loggedoff

Hey!

Hatte einen Teil des AntiCheat-Codes nicht auskommentiert gehabt, sorry.
Es liegt anscheinend wirklich an der Geschwindigkeit meines Codes... aber so viel mache ich garnicht, lediglich ein paar Abfragen. Es wird sozusagen langsam aber doch entsynchronisiert? Das ist schlecht, sehr schlecht. Sollte DirectPlay das nicht regeln?

Trotzdem ist es so, dass mein Code ohne CriticalSection schon seit Stunden läuft, auch mit vielen Spielern. Sobald ich eine CS einbaue, hängt sich der Server kurze Zeit später auf.
Aber ich denke das passiert deshalb weil dann immer nur ein thread dran ist, was wieder viel Zeit kostet...

Was mir noch aufgefallen ist:
Wenn ich ein Sleep(1) einbaue (ohne CS) und die DLL, in dem der ganze Code steckt, schnell und oft hintereinander lade und wieder entlade, stürzt der Server einfach ab, also das Programm schließt sich. Ohne Sleep() nicht. Woran könnte das liegen?

Was kann ich tun? Ich kann auf einen AntiCheat nicht verzichten
Gibt es da Möglichkeiten?

rapso schrieb:

es gibt probleme sobald die map modifiziert wird. nicht nur wenn gerade zwei funktionen gleichzeitig aufgerufen werden, auch sonst kann der iterator invalidiert werden.

Also hatte ich paar Stunden lang einfach nur Glück? Hmm... wo ist genau das Problem? Intern in std::map? Pro Callback wird ein eigener Iterator angelegt, da sollte es also kein Problem geben...
Ich denke, ich muss ohne CS auskommen, da der Server es wohl nicht mag, wenn ein callback zu lange warten muss... (Wird ja dann alles auch nur halb so schnell ausgeführt bei 2 callbacks, wenn einer warten muss)

MfG