Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)"

[hustbaer]

Lol
Sorry, aber wenn du uns nix glaubst, wieso sollten wir dann weiter versuchen dir zu helfen.

Das

Das (Zeile 23, erster Codeteil) kann es eigentlich nicht sein.
Es müsste dann ja an dieser Stelle krachen.
ist halt einfach nicht wahr.

Und das

Die map muss zwangsläufig gleichzeitig gelesen und bearbeitet werden,
sonst funktioniert die ganze Sache nicht.
auch nicht. Du kannst vor jedem Zugriff locken, dann den Zugriff machen und dann unlocken. Das machen quasi alle C++ Programme so. Oder du begibst dich auf die Suche nach einer concurrent map für C++.

So. Nu kannst du aus dem was dir geschrieben wurde was machen, oder du kannst es weiterhin ignorieren und behaupten dass das nicht der Grund sein kann weil blah. Mir wörst. Ich bin auf jeden Fall hier raus.

[elmut19]

@hustbaer
Auf jeden Fall vielen Dank Euch allen.
Ich werd das mit den zusätzlichen locks versuchen.
Auch wenn es mir seltsam erscheint. Verlieren kann ich ja nix dabei.
Werde dann berichten.
Was alles im Hintergrund dabei abgeht, in meiner Version, würde ich auch gerne verstehen.

[elmut19]

Habe zur Sicherheit noch etwas nachgelesen, z.B. bei Stackoverflow zu "race conditions" "std::map"
Natürlich kann ich mich immer noch irren.
Aber ich bin weiterhin davon überzeugt, dass mein Hauptfehler nicht die CritSec sind.
Den Schutz der Elemente in der "map" habe ich sehr wohl bedacht!
Und wenn ich ohne Absicherung ein Element lösche, auf das ein Iterator noch zeigt, kommt anderer Fehler.
"erase(key)" und "insert(..)" habe ich zudem auch auf "map"-Ebene abgesichert. Also schon Einiges ergänzt.
Nun habe ich mein Programm noch so eingestellt, dass nur noch ein Thread läuft, der auch mit nur einer
der "maps" arbeitet. Ich kann also von vornherein Zugriffskonflikte ausschliessen.

Wenn ich das Objekt beenden will, dem die maps gehören, so stürzt es ab, und das obwohl die maps
zuvor ordentlich gelöscht wurden (erase(key)). Gelöscht von dem Thread, der die Elemente gespeichert hat.

Die maps enthalten, in der "struct", auf die sie als ihr Element verweisen, selbst komplexe Objekte,
die allerdings zu dem Objekt gehören (also die komplette "struct"), das den TCP-Thread startet.
Also zum Zeitpunkt, wo dieses Basis-Objekt an sein Ende kommt, existieren die Objekte, die die
Threads starteten, sowie deren "structs" nicht mehr. Aber sie wurden auch vorher schon bereinigt.

Also die "maps" enthalten als Element nur Verweise auf eine "struct".
Und es kommt in diese "void _Orphan_ptr(const _Nodeptr _Ptr) noexcept" aus den STL-Methoden.

Hat niemand eine Idee?

[DocShoe]

Dieser Beitrag wurde gelöscht!

[elmut19]

Bin zu dem Schluss gekommen, dass sich eine "std::map" nicht mit einer "struct", die selbst komplexe Objekte enthält, verträgt.
Auch wenn ich nur einen NULL-Pointer in meine "map" reinschreiben und vor Ende meines Basis-Objekts alles aus der "map" bereinigt wurde,
gehts schief. Und der Thread, der als einziger mit dieser "map" gearbeitet hat, ist auch schon ordentlich beendet und hat dabei sein "map"-Element gelöscht.

Ich werd wohl ne eigene "map" schreiben müssen.
Danke aber nochmals für Eure Hilfe.

[It0101]

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Ich werd wohl ne eigene "map" schreiben müssen.

So hab ich auch mal gedacht, das ist aber eine Sackgasse. Container kann man mal zu Lernzwecken implementieren, aber nicht zur ernsthaften Verwendung. Die STL-Container sind für ihren jeweiligen Anwendungsfall das beste was du bekommen kannst.
Bei uns im Unternehmen gab es in der pre-C++11-Zeit auch viele selbstimplementierte Container, aber das war alles fürchterlich. Ich habe dann nach und nach nachgewiesen, dass die Performance der STL erheblich besser ist und die Usability auch. Seitdem kam keiner mehr auf die Idee sowas selber zu schreiben.

Wenn die Map nicht zu deinem restlichen Konzept passt, dann ist vermutlich dein restliches Konzept ungeeignet und nicht die Map.

Was das "struct" angeht, sehe ich das ähnlich. Das funktioniert zwar, aber ich vermeide das auch. "structs" sind bei mir entweder PODs oder befördere sie zu einer Klasse.

[Tyrdal]

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Bin zu dem Schluss gekommen, dass sich eine "std::map" nicht mit einer "struct", die selbst komplexe Objekte enthält, verträgt.

Da liegst du völlig falsch.

Auch wenn ich nur einen NULL-Pointer in meine "map" reinschreiben und vor Ende meines Basis-Objekts alles aus der "map" bereinigt wurde,
gehts schief. Und der Thread, der als einziger mit dieser "map" gearbeitet hat, ist auch schon ordentlich beendet und hat dabei sein "map"-Element gelöscht.

Klingt als hättest du dir irgendwo den Speicher "versaut".

Ich werd wohl ne eigene "map" schreiben müssen.

Das halte ich wiederum für einen groben Feher.

[DocShoe]

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Ich werd wohl ne eigene "map" schreiben müssen.

Da kann ich mich meinen Vorrednern nur anschließen, die STL oder vergleichbare boost-Container sind das Beste, was du bekommen kannst. Daran arbeiten Leute, die den ganzen Tag nichts anderes machen, was Selbstprogrammiertes wird an die Qualität wohl selten herankommen.
Das Problem ist aber nicht die map selbst, sondern etwas anderes (die Vermutung race condition steht immer noch im Raum). Und das wirst du auch mit einer selbstprogrammierten Map nicht lösen können.

Lass mal einen statischen Code Analyser über deinen Quelltext laufen (zB cppcheck), die finden oft Ungenauigkeiten/Fehler. Ansonsten führ´ dein Programm in einem Profiler aus, der Speicheranalysen durchführt (check auf double frees, Schreiben in nicht-initialisierten Speicher, etc.). Vielleicht hilft dir das weiter.

[DocShoe]

@It0101 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Ich werd wohl ne eigene "map" schreiben müssen.

Bei uns im Unternehmen gab es in der pre-C++11-Zeit auch viele selbstimplementierte Container, aber das war alles fürchterlich. Ich habe dann nach und nach nachgewiesen, dass die Performance der STL erheblich besser ist und die Usability auch. Seitdem kam keiner mehr auf die Idee sowas selber zu schreiben.

Da habe ich ein Deja-Vue. An Teil 1 arbeite ich noch (also Überzeugung, nicht eigene Container), aber den Mythos "std::vector ist langsam" und "std::vector" ist für viele Elemente nicht geeignet" hält sich hartnäckig :(.

[Quiche Lorraine]

@Tyrdal sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Klingt als hättest du dir irgendwo den Speicher "versaut".

Das vermute ich schon ein wenig länger, weiß aber nicht so recht wie ich es schreiben soll.

Ich probiere es mal.

@elmut19

Du nutzt meiner Meinung nach zu viele Pointer (z.B. pCRing, PMRDATA, PMRCANIDSRAM, std::map<CString,PMRCANIDSRAM>, std::map<CString,PMRDATA>* m_pmapData) und dadurch könnten die Verantwortlichkeiten nicht sauber definiert sein.

So könnte es z.B. sein das MRDATA::pWriteRsp an irgenteine Stelle gelöscht wird und es einen Destruktor für MRDATA gibt, welche pWriteRsp nochmals löschen will.

Aus meiner Sicht müsste man deswegen den Code aufräumen und die Verantwortlichkeiten in Klassen packen. Warum z.B. muss pWriteRsp von Hand gelöscht werden? Warum kann das nicht die Klasse CRing bzw. MRDATA tun bzw. verantwortlich dafür sein?

Such dir bitte mal jede Stelle wo du einen Pointer nutzt und stelle dir die folgenden Fragen?

• Handelt es um einen Zeiger, welcher Ressourcen allokiert?
• Wenn ja, wer ist für das Löschen verantwortlich? Wer für das Allokieren?
• Sind Zugriffe auf den Zeiger immer sicher?
• Welche Querabhängigkeiten entstehen durch die Verwendung des Zeigers?
• Kann man anstatt des Zeigers auch eine Kopie nutzen?

Schau dir noch mal MRDATA::pWriteRsp an. Durch die Verwendung eines Zeiger ist die Lebenszeit von pWriteRsp nicht unbedingt an die Lebenszeit der MRDATA Instanz gebunden. Du hast in der UML Sprache hier eine Relation. Würdest du gegegen bei MRDATA::pWriteRsp keinen Zeiger nutzen, so wäre die Lebenszeit von pWriteRsp an die Lebenszeit der MRDATA Instanz gebunden. Du hättest in der UML Sprache eine Aggregation. Ähnliches gilt für std::map<CString,PMRDATA> m_mapData;.

[elmut19]

@Quiche-Lorraine
Ja. Ich halte die stl auch für genial!
Halte deshalb auch schon seit fünf Wochen an dem Konzept fest.
Habe auch schon an alle möglichen Stellen gedacht, wo noch mein Pointer manipuliert werden kann.

Ich habe das Ganze schon soweit manipuliert, dass ich in der "map" nur noch einen "NULL-Pointer" habe und meinen bisherigen Pointer für die Operationen verwende.
Von der "map" brauche ich dann nur den Key, damit ich weiss wer nach den Daten fragt. Und ich habe sichergestellt, dass auch nur der eine Kandidat mit dem "NULL-Pointer" etwas in die "map" geschrieben hat.

"zu viele Pointer (z.B. pCRing, PMRDATA, PMRCANIDSRAM, st"
Hierfür sorry. PMRDATA ist identisch mit PMRCANIDSRAM.
Ich hab es anfangs im Text nur umbenannt. In Fehlerausgabe ist das Original.
Es ist der Datentyp der "struct", die als Element in der "map" ist.
Und "pCRing" ist Datentyp von drei gleichartigen Elementen in dieser "struct"

Diese "struct" ist das zentrale Kommunikations-Objekt,
über das das HMI über den TCP-Client-Thread mit der Anlage kommuniziert.
Die "map" vervielfacht diese "struct", damit die Daten auf mehrere HMI´s verteilt werden können.
Das war auch die Idee zu dem gewünschten Feature, aus einer einzigen Client-App einen Verteiler zu basteln,
über den die Anlage nur noch eine TCP-Verbindung für mehrere Clients halten muss.
Genaugenommen ist das schon die einzige Änderung.
Soweit habe ich zumindest alles reduziert. Also sogar ohne zusätzliche Clients.

Damit sollte doch eigentlich alles was ausserhalb noch so allokiert wird ausgeschlossen sein.
An das rechtzeitige Beenden der Threads, einschliesslich Aufräumen der Objekte habe ich auch gedacht.
Und ja, normalerweise wird auch allokiert (CRing).

Aus der "struct" eine "class" zu machen, habe ich auch schon gedacht und damit angefangen.
Hat nicht auf Anhieb geklappt. Hängt viel anderes Zeug mit dran.
Wäre evtl. wieder einen Versuch wert.

[Quiche Lorraine]

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Hierfür sorry. PMRDATA ist identisch mit PMRCANIDSRAM.

Das P... ist doch ein Windows spezifischer Präfix für einen Pointer. Deswegen mein Hinweis und die Frage: Funktioniert nicht hier auch ein MRDATA bzw. MRCANIDSRAM.

Versuche mal bitte das Problem dahinter zu verstehen. Wenn man Zeiger nutzt, so kann dies unerwartete Seiteneffekte haben, weil da eben eine Relation ist.

Genau deswegen gefallen mir Klassen. Man kann diese so entwickeln das sie Problem kapseln, einfach zu benutzen und schwer falsch zu benutzen sind.

Deswegen wäre mein erster Schritt MRDATA folgendermaßen uumzubauen

class MRDATA_2
private:
public:
	bool				bInUse;
	CRing				ReadRsp;
	CRing				WriteRsp;
	CRing				ErrorRsp;
	long				lWritePos;
	DWORD				lWriteRspPos;
	bool				bWriteToCan;
	bool				bDataInWRITE;
	HANDLE			hTimerThreadRspSleep;
	CRITICAL_SECTION	m_csRspCriticalSection;  // evt. C++ Äqauivalent
};

Ein MRDATA besteht so ReadRsp, WriteRsp und ErrorRsp Ringpuffer,...

Ferner würde CRing sich eigenständig um die Ressourcenverwaltung kümmern. Ich habe hierzu mal ein std::array benutzt als ich ein Ringpuffer-Template geschrieben habe.

Und dann versuche doch mal einen Fehler in MRDATA_2 zu verursachen und danach mit MRDATA.

[elmut19]

@Quiche-Lorraine
Hallo Quiche-Lorrain,
Vielen Dank noch für den Hinweis.
Aber hier auch ja, ich habe es probiert. Vielleicht habe ich nicht alle Optionen
mit dieser Variante durchgespielt, aber es endete mit dem gleichen Fehler.
Ich sehe es aber weiterhin noch als Option, da nochmal dran zu arbeiten.

Ich bin aber momentan noch bei der Variante, die "struct" in eine "echte" Klasse umzubauen.
Bisher habe ich nur mal das "struct" gegen "class" getauscht, ohne irgendwelche Konstruktoren oder Destruktoren einzubauen.
Vielleicht verliert der Compiler durch das "struct" den Bezug zu internen
Destruktoren der Elemente und vermutet daher dort vergessenen Objekte,
weshalb dann der Fehler ausgelöst wird.

[Th69]

Nein, ein struct X ist nichts anderes als

class X
{
public:
   // ...
}

(also public als Standardzugriffsmodifizierer - als auch beim Ableiten, aber das nutzt du ja nicht).
Du kannst dort ebenfalls Konstruktoren, Destruktoren sowie Memberfunktionen implementieren.

Mit "Umbau zur Klasse" ist nur gemeint, daß du nicht nur eine reine Datenstruktur hast, welche von außen manipuliert wird, sondern auch Memberfunktionen, welche sich um den internen Zustand kümmern (Validierung, De-/Allokationen, ...).

Und deine Problembeschreibung im Eingangsbeitrag deutet wirklich darauf hin, daß der Speicher der Anlage-Klasse irgendwie überschrieben wurde, so daß die map-Objekte falsche Zeiger beinhalten. Gerade das Crashen auch bei einem leeren map zeigt ja, daß es nicht an den enthaltenen Daten liegen kann.
Verschieb doch mal diese map-Objekte an den Anfang oder Ende deiner Anlage-Klasse (ich nehme mal an, du hast noch mehr als diese zwei Member).

Oder zeige mal die Member der Anlage-Klasse, sowie dessen Destruktor.

PS: Du kannst es auch mal mit einem Datenhaltepunkt (data breakpoint) auf das map-Objekt versuchen, um Schreibänderungen zu finden: Verwenden von Breakpoints im Visual Studio-Debugger: Festlegen von Datenbreakpoints (nur nativer C++-Code)

[DocShoe]

Ich kann mich da nur wiederholen:

• mach eine statische Codeanalyse, zB mit cppCheck, um Fehler im Programm zu finden
• benutz einen Profiler, um ungültige Schreibzugriffe in Speicher zu finden.

Undefined Behaviour zu finden und zu beseitigen ist so das Unangenehmste, was man machen muss.

[It0101]

@DocShoe sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

@It0101 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

@elmut19 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Ich werd wohl ne eigene "map" schreiben müssen.

Bei uns im Unternehmen gab es in der pre-C++11-Zeit auch viele selbstimplementierte Container, aber das war alles fürchterlich. Ich habe dann nach und nach nachgewiesen, dass die Performance der STL erheblich besser ist und die Usability auch. Seitdem kam keiner mehr auf die Idee sowas selber zu schreiben.

Da habe ich ein Deja-Vue. An Teil 1 arbeite ich noch (also Überzeugung, nicht eigene Container), aber den Mythos "std::vector ist langsam" und "std::vector" ist für viele Elemente nicht geeignet" hält sich hartnäckig :(.

Ich habe solche Diskussionen immer damit erstickt:
https://i.stack.imgur.com/G70oT.png

Manch einer verwendet nachwievor den falschen Container für den falschen Anwendungsfall.

[It0101]

@Quiche-Lorraine sagte in [Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)"]

Genau deswegen gefallen mir Klassen. Man kann diese so entwickeln das sie Problem kapseln, einfach zu benutzen und schwer falsch zu benutzen sind.

Insbesondere, wenn man ihre Kopierfähigkeit/Bewegungsfähigkeit einschränkt. Auf die Art hab ich schon so manches Ei gefunden, weil manche Dinge dann unerwartet doch irgendwie kopiert wurden.

[Quiche Lorraine]

@Th69 sagte in Zugriffsfehler auf "std::map" in " _Orphan_ptr(const _Nodeptr _Ptr)":

Mit "Umbau zur Klasse" ist nur gemeint, daß du nicht nur eine reine Datenstruktur hast, welche von außen manipuliert wird, sondern auch Memberfunktionen, welche sich um den internen Zustand kümmern (Validierung, De-/Allokationen, ...).

Genau.

Denn mit einer gut designten Klasse reduziert man einiges an Code-Komplexität.

Und das versuche ich @elmut19 zu erklären. Viel C Code ist offener Natur und funktioniert nur deswegen gut, weil man sich sehr diszipliniert an Regeln hält.

Mit C++ wird das wesentlich einfacher, da man dank RAII/STL sich fast nicht mehr um Ressourcenverwaltung kümmern muss und dank den Klassen Kapselungen besser implementieren kann.

So richtig verstanden habe ich dies aber erst, als ich jede Menge C Code nach C++ portierte.

PS:
@elmut19

Warum nicht so?

// Democode, nicht lauffähig
class MRDATA_2
private:
  bool				bInUse;
  CRing				ReadRsp;
  CRing				WriteRsp;
  CRing				ErrorRsp;
  long				lWritePos;
  DWORD				lWriteRspPos;
  bool				bWriteToCan;
  bool				bDataInWRITE;
  HANDLE			hTimerThreadRspSleep;
  CRITICAL_SECTION	m_csRspCriticalSection;  // evt. C++ Äqauivalent

public:
  void GetReadRsp(CRing& ReadResponse)
  {
    // EnterCriticalSection 
    // ReadResponse = this.ReadRsp
    // LeaveCriticalSection
  }

  // ...
};

Deine ganzen Daten sind private. Du kommst also außerhalb der Klasse nicht mehr an deine Daten ran. Also musst du z.B. die Funktion GetReadRsp() nutzen und diese nutzt automatisch deine CRITICAL_SECTION Instanz. Sofern deine Klasse korrekt implementiert ist, zwingst du dich also automatisch zur Thread Safety.

[elmut19]

@Quiche-Lorraine
Vielen Dank nochmal für Eure Vorschläge.
Ich brauch nun eine Weile, um mir das alles nochmal durch den Kopf gehen zu lassen.
Ich werd mal mit einer vollständigen Klasse statt der "struct" (bzw. einfach nur "class" zu schreiben) weitermachen.
Und dann noch die Sache ohne Pointer. Davon hab ich schon ne Version als "struct".
Das ganze Anlage-Objekt ist wirklich ein Mega-Teil und kaum zu überblicken.
Vorerst halte ich noch Abstand, das hier abzudrucken.
Habe zudem noch ein ganz anders Problem reinbekommen, um das ich mich auch noch kümmern muss

[Th69]

Der Fehler liegt nicht an der MRDATA-Datenstruktur!
Das ganze sollte doch mit ein paar Minuten Debugging gelöst sein...

Oder generell, wie schon von @DocShoe vorgeschlagen, CppCheck benutzen - für VS das passende CppCheck Add-In.