Zugriffsverletzung in BORLNDMM.DLL bei std::list

Hallo,

ich verwende desöfteren folgendes typedef

typedef std::liststd::string ListeStrings;

Programme, die mit Turbo C++ 2006 kompiliert wurden, stürzen leider deswegen sporadisch ab. Im DrWatson.log kann ich dann folgendes lesen:

*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\DEV\LE32\BIN\BORLNDMM.DLL - 
Funktion: BORLNDMM!Borlndmm
        216742ca 89f0             mov     eax,esi
        216742cc 5f               pop     edi
        216742cd 5e               pop     esi
        216742ce 5b               pop     ebx
        216742cf c3               ret
        216742d0 5b               pop     ebx
        216742d1 85c0             test    eax,eax
        216742d3 0f89fbfbffff     jns     BORLNDMM!ReallocMemory+0x768 (21673ed4)
        216742d9 31c0             xor     eax,eax
        216742db c3               ret
FEHLER ->BORLNDMM!Borlndmm:
216742dc 8b50fc           mov     edx,[eax-0x4]     ds:0023:000001f0=????????
        216742df f6c207           test    dl,0x7
        216742e2 89c1             mov     ecx,eax
        216742e4 53               push    ebx
        216742e5 8a1da0776721     mov     bl,[BORLNDMM+0x77a0 (216777a0)]
        216742eb 0f85cb000000     jne     BORLNDMM!Borlndmm+0xe0 (216743bc)
        216742f1 84db             test    bl,bl
        216742f3 8b1a             mov     ebx,[edx]
        216742f5 7561             jnz     BORLNDMM!Borlndmm+0x7c (21674358)
        216742f7 836a0c01         sub     dword ptr [edx+0xc],0x1
        216742fb 8b4208           mov     eax,[edx+0x8]

*----> Stack Back Trace <----*
*** WARNING: Unable to verify checksum for C:\DEV\LE32\CC3270MT.DLL
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\DEV\LE32\BIN\CC3270MT.DLL - 
WARNING: Stack unwind information not available. Following frames may be wrong.
ChildEBP RetAddr  Args to Child              
0c08fdfc 3274fc45 000001f4 0c08fe3c 32709302 BORLNDMM!Borlndmm
0c08fe08 32709302 000001f4 00ac3bb0 0c08fe84 CC3270MT!free+0xd
0c08fe3c 00df252f 000001f4 0c08fe60 00df22c6 CC3270MT!$bdele$qpv+0x1e
0c08fe48 00df22c6 0c893a70 000001f4 0ce845c8 LE32+0x252f
0c08fe60 00df1bd5 0c893a68 00000001 00000000 LE32+0x22c6
0c08fe74 00e5cc42 0c893a68 00000002 0c08fef8 LE32+0x1bd5
....

Ich konnte das Problem soweit nachvollziehen, als dass dieser Fehler (sporadisch) auftritt, wenn der Destruktor eines Objekts, welches so eine Liste als Klassenmember enthält, aufgerufen wird, d.h. das Problem tritt auf, wenn ein solches ListeStrings Objekt zerstört wird.

Leider konnte ich das aber nicht auf einem Entwicklungsrechner nachvollziehen. Lässt man das Programm in der IDE im Debugger laufen, tritt das Problem jedenfalls nicht auf. Startet man den selben Build Standanlone, dann relativ schnell sofort.
Ich habe ein Testprogramm geschrieben welches in mehreren Threads solche Objekte erstellt und wieder zerstört, weil ich dachte vielleicht sind die std::strings reference counted und es gibt damit Probleme. Aber das Programm läuft einwandfrei.

Jetzt natürlich die Frage: Wodran kann das liegen?

audacia

std::string ist beim C++Builder nicht referenzgezählt. AnsiString/UnicodeString benutzen Referenzzählung und copy on write, sie sind allerdings auch threadsicher.

Das könnte natürlich ein subtiles Problem mit dem Compiler oder Linker des C++Builder 2006 sein. Wäre es dir möglich, ein Projekt zu posten, das das Problem einigermaßen verläßlich reproduziert? Ich habe zwar C++Builder 2006 nicht mehr installiert, aber dann könnte man wenigstens andere Fehlerquellen ausschließen.

Wäre es dir möglich, ein Projekt zu posten, das das Problem einigermaßen verläßlich reproduziert?

Leider nicht. Mit meinem Testprogramm kann ich es ja auch nicht reproduzieren obwohl es eigentlich genau das selbe tut, wie das eigentliche Programm.
Leider werden da auch gar nicht so komplizierte Aktionen durchgeführt: Im wesentlichen erzeugt ein Thread Nachrichten-Objekte, die halt je nach Typ so ein ListeStrings als Member haben. Es wird gefüllt, dann auf einen anderen Thread in eine mit CriticalSections geschützte Queue gepostet. Der andere Thread holt sich die Nachricht raus, verarbeitet sie und löscht dann das Objekt.

Ich werde jetzt mal testweise das Typedef auf std::vectorstd::string ändern.

audacia

TurboC++ schrieb:

Leider werden da auch gar nicht so komplizierte Aktionen durchgeführt: Im wesentlichen erzeugt ein Thread Nachrichten-Objekte, die halt je nach Typ so ein ListeStrings als Member haben. Es wird gefüllt, dann auf einen anderen Thread in eine mit CriticalSections geschützte Queue gepostet. Der andere Thread holt sich die Nachricht raus, verarbeitet sie und löscht dann das Objekt.

Das klingt allerdings eher nach einem Synchronisationsproblem (race condition) als nach irgendwelchen Compilerbugs.

Kannst du mal allen Code posten, der direkt auf die Stringlisten zugreift, inklusive der Absicherung durch die Critical Section?

Es wird jeweils nur von einem Thread auf die Objekte zugegriffen, deswegen sind die Stringlisten selbst nicht geschützt.
Es gibt einen "Haupt"-thread, der die Events die von verschiedenen Clients erhält und geordnet abarbeiten muss.
Die Clients erzeugen ein Event mit "new KommEvent", füllen es mit Daten und posten es dann mit EnqueueEvent in die Queue. Der Hauptthread ruft in seiner Execute-Methode unter anderem die Deliver Methode der unten genannten Klasse auf, die dann ihrerseits versucht die Events zuzustellen.

CSLock ist eine Hilfsklasse, die nichts anderes macht als in ihrem Konstruktor EnterCriticalSection und im Destruktor LeaveCriticalSection aufzurufen.

class KommEvent; // Forward declaration of Event
typedef std::queue<KommEvent*> EventQueue;

class KommunikationsEvents
{
private:
 EventQueue       Events;
 CRITICAL_SECTION CSGuard;

public:
 KommunikationsEvents(void);
 ~KommunikationsEvents(void);

 void       EnqueueEvent(KommEvent *pEvent );
 void       Deliver(void);
};

void   KommunikationsEvents::EnqueueEvent(KommEvent *pEvent)
{
 CSLock lock(CSGuard);
 Events.push(pEvent);
}

void KommunikationsEvents::Deliver(void)
{
 CSLock lock(CSGuard);
 EventQueue UndeliveredEvents;

 while (!Events.empty())
 {
  KommEvent *pEvent = Events.front();
  if (pEvent->Execute())
  {
   delete pEvent;
  }
  else
  {
   UndeliveredEvents.push(pEvent); // Event merken für nächstes mal
  }
  Events.pop();
 }

 // Events MUSS nun leer sein, sonst hätte while nicht beendet. Aber wir
 // müssen nun die Events die eben nicht bearbeitet werden konnten
 // wieder in die Queue eintragen, damit sie beim nächsten Zyklus nochmal
 // probiert werden.
 Events = UndeliveredEvents;
}

Ein Event Objekt sieht so aus

class EventPosition : public KommEvent
{
public:
 EventPosition(int iStation, const ListeStrings &lIds) : 
  miStation(iStation), mlIds(lIds) {}:
 virtual ~EventGlassPosition(void) {};

 virtual bool Execute(void);

protected:
 int                miStation;
 ListeStrings       mlIds;
};

In der Execute Methode wird eine Manipulation am Datenmodell durchgeführt, welche eben nur vom Hauptthread aus erlaubt sind. Danach wirds - wie oben zu sehen - gelöscht und dabei tritt sporadisch die Exception auf.

audacia

Du rufst natürlich schon auch InitializeCriticalSection() und DeleteCriticalSection() auf, oder?

TurboC++ schrieb:

void KommunikationsEvents::Deliver(void)
{
 CSLock lock(CSGuard);
 EventQueue UndeliveredEvents;

 while (!Events.empty())
 {
  KommEvent *pEvent = Events.front();
  if (pEvent->Execute())
  {
   delete pEvent;
  }
  else
  {
   UndeliveredEvents.push(pEvent); // Event merken für nächstes mal
  }
  Events.pop();
 }

 // Events MUSS nun leer sein, sonst hätte while nicht beendet. Aber wir
 // müssen nun die Events die eben nicht bearbeitet werden konnten
 // wieder in die Queue eintragen, damit sie beim nächsten Zyklus nochmal
 // probiert werden.
 Events = UndeliveredEvents;
}

Das ist nicht exceptionsicher. Wenn in KommEvent::Execute() eine Exception geworfen wird, wird deine Critical Section verlassen und die "UndeliveredEvents" verworfen (und nie freigegeben). Außerdem wird das aktuelle Event später möglicherweise erneut ausgeführt. Es kann auch UndeliveredEvents.push() eine Exception werfen - dann würde auch das übergebene Event "vergessen".
Allerdings sehe ich im Moment - vorausgesetzt, Execute() löst keine Exception aus - keine Ursache für eine Race-Condition. Meine Erfahrung mit solchen Problemen ist aber beschränkt, also will das nicht viel heißen.

Eines der Grundprobleme ist, daß du mit der Queue eine unpassende Datenstruktur verwendest. Sicher, das grundlegende Benutzungsmodell entspricht dem, was du willst - vorne rein, hinten raus -, aber zusätzlich gibt es die folgenden Anforderungen:
1. Jedes nicht "erfolgreiche" Event wird beim nächsten flush erneut ausgelöst, muß sich also wieder in der Queue hinten anstellen.
2. Jedes "erfolgreiche" Event muß freigegeben werden.
3. Wenn eine Exception auftritt, sollte das jeweilige Event nicht mehr ausgeführt (sonst bekommst du Exceptions ad infinitum), sondern direkt freigegeben werden.
4. Vermutlich willst du, daß eine Exception die Queue-Abarbeitung unterbricht und normal propagiert wird, so daß der Benutzer den üblichen Exception-Dialog angezeigt bekommt.

Punkt 4 erfordert, daß du im Exceptionfall die noch abzuarbeitenden Elemente - also die verbleibenden Elemente in "Events" und alle Elemente in "UndeliveredEvents" in eine einzige Queue - nämlich "Events"- zusammenführst. Dummerweise mußt du dafür Speicher anfordern, und dabei kann eine Exception auftreten - aber eine Exception, die während der Behandlung einer anderen Exception auftritt, terminiert das Programm!

Eine Möglichkeit wäre, stattdessen eine std::list<> zu benutzen, mit push_back() Events anzuhängen, die Events von vorne nach hinten durchzugehen und dabei "nicht erfolgreiche" Events einfach drinzulassen. Etwa so:

typedef std::list<KommEvent*> EventQueue;
...
void KommunikationsEvents::Deliver(void)
{
  CSLock lock (CSGuard);
  EventQueue::iterator i = Events.begin ();
  while (i != Events.end ())
  {
    try
    {
      if ((*i)->Execute ()) // alles nach dieser Anweisung wirft keine Exceptions
      {
        EventQueue::iterator old = i++;
        delete *old;
        Events.erase (old); // old ist jetzt ungültig, deshalb mußten wir vorher
                            // zum nächsten Element weitergehen
      }
      else
        ++i; // Element in der Liste belassen
    }
    catch (...)
    {
      EventQueue::iterator old = i++;
      delete *old;
      Events.erase (old);
      throw; // Exception weiterwerfen
    }
  }
}

Wirklich gut wird es allerdings erst, wenn du so etwas diffiziles wie Threadsynchronisationscode nicht selbst schreibst, sondern eine bereits fertige Implementation nimmst. Dies aufgrund der Tatsache, daß es großer Erfahrung bedarf, Code zu schreiben, der sich in Multithreading-Situationen immer korrekt verhält, und die empirisch belegte Feststellung, daß die wenigsten über diese Erfahrung verfügen. Das soll kein Affront gegen deine Fähigkeiten sein; für mich gilt das natürlich gleichermaßen, und auch ich würde obigen Korrekturvorschlag nicht produktiv einsetzen, sondern stattdessen eine fertige Lösung benutzen.

Insbesondere ermöglicht der Einsatz einer lock-free queue den Verzicht auf eine critical section und damit einen Synchronisationsflaschenhals. Zu dem Thema ist das hier vielleicht interessant.

Wie es beim C++Builder 2006 mit externen Lösungen aussieht, weiß ich nicht, aber in C++Builder XE hat TThread die folgenden Methoden:

class procedure Queue(AThread: TThread; AMethod: TThreadMethod); overload; static;
    ...
    class procedure Queue(AThread: TThread; AThreadProc: TThreadProcedure); overload; static;

Soweit ich es überblicken kannn, tun diese Methoden ziemlich genau das, was du willst. Die Überladung für TThreadProcedure (den es erst ab D2009 gibt) kannst du sogar für Lifetime-Management benutzen; in Delphi übergibt man eine anonyme Methode, in C++ implementiert man ein Interface.

Ansonsten gibt es auch weitere ausgereifte Threading-Lösungen, etwa die Omni Thread Library oder irgendwas aus Boost. Wie weit du damit allerdings in C++Builder 2006 kommst, kann ich nicht sagen.

Im Zweifelsfall gibt es natürlich immer die TThreadList, die recht einfach grundlegende Threadsynchronisation ermöglicht.

Hallo!

Erstmal Danke für deine ausfürliche Antwort.

Du rufst natürlich schon auch InitializeCriticalSection() und DeleteCriticalSection() auf, oder?

Natürlich. Ich habe habe nur darauf verzichtet Konstruktor und Destruktor der Klasse zu posten.

Das ist nicht exceptionsicher. Wenn in KommEvent::Execute() eine Exception geworfen wird.

Dieser Code ist nicht exceptionsicher, weil er das nicht sein muss (bzw. dann oftmals die Zeit fehlt, sich um Schönheit zu kümmern): Es handelt sich hier um ein Steuerungsprogramm, welches ohne Benutzerinteraktion läuft. Wenn eine Exception beim bearbeiten der Events auftritt, wird diese in der Execute Methode des Hauptthreads gefangen, geloggt und das Programm beendet. Mir bleibt da keine andere Wahl, denn wenn da irgendwo eine Exception auftritt, bedeutet das leider sowieso einen Programmierfehler. Nichtsdestotrotz werd ich mir mal eine lock-free queue ansehen

Zurück zum Thema: Ich habe dieses "fangen" im Hauptthread im Moment rausgenommen, damit das Programm "richtig" crasht und ich ein DrWatson.log erhalte. Sonst erhalte ich in meinem Log nämlich nur den Hinweis, das eine AccessViolation aufgetreten ist. Erst so bin durch die Analyse der Stack-Traces darauf gekommen, dass es beim Zerstören der ListeStrings Klassenmember auftritt.

Mit dem typedef geändert auf vector ist es übrigens bisher nicht mehr aufgetreten.

Noch ein Nachtrag:

IMHO ist dein Beispiel-Code nicht ganz korrekt. Man kann doch nicht einfach

[code]
EventQueue::iterator old = i++;
delete *old;
Events.erase (old);
[quote]

machen?! Eventuell steht man doch dann nach dem i++ auf .end() and macht dann darauf das delete.

Vergiss es Ich glaube einfach nichts bevor ich nicht verstanden habe