CopyStructure(): Nicht Threadsicher auf "unschöne Art".

[cxAlex]

CopyStructure() ist auf eine sehr "unschöne" Art nicht Threadsicher: Befinden sich in der Source-Struktur dynamische Elemente wie Maps, Listen, Arrays werden diese nicht vollständig oder auf irgend eine Art sogar "übermäßig" kopiert wenn mehrere Threads von der selben Quelle kopieren. In diesem Beispiel wird eine Map mit 100 Einträgen kopiert, wobei die Ziel-Struktur dann eine Map mit 0- ~150 Einträgen enthalten kann, ohne jedoch den Heap zu korrumpieren oder das Programm irgendwie anderweitig abstürzen zu lassen. Es kann auch vorkommen das die Anzahl der Elemente übereinstimmt, dann aber trozdem Elemente fehlen . Das ganze ist einfach testbar mit folgendem Code, per Compilerswitch kann das ganze dann Thread-sicher gemacht werden, zum Vergleich.

Man könnte jetzt natürlich meinen "Das ist kein Bug, das ganze ist einfach nicht Thread-Safe, musst du halt selbst absichern...".
Trifft aber hier IMHO nicht zu. Das ganze ist doch ein sehr schwer zu findender Fehler, und speziell Anfänger von diesem Verhalten sicher verwirrt sind, den sobald jemand fragt ob er in einem Thread nun etwas schützen muss ist so ziemlich die 1. Aussage "Solange alle Thread nur vom selben lesen dann nicht, gemeinsame Schreibzugriffe schon". Und das trifft hier einfach nicht zu. Außerdem könnte man die Thread-Sicherung viel performanter innerhalb der CopyStructure() Procedure einbauen anstatt sie zu ummanteln, da man ja nur die dynamischen Teile der Struktur schützen müsste, die statischen Einträge hingegen nicht.

Code: Alles auswählen

EnableExplicit

#NB_Elements = 100
#NB_Threads = 100

; Switch to Test:
#LockCopy = #False

Structure myStruct
  Map MyMap.i()
EndStructure

Global myGlob.myStruct
Define i

For i = 1 To #NB_Elements
  myGlob\MyMap(Str(i)) = i
Next

Global _TMMMutex = CreateMutex()
CompilerIf #LockCopy
  Global _CopyMutex = CreateMutex()
CompilerEndIf

Macro ThreadModalMessageBox(_Title, _Text)
  LockMutex(_TMMMutex)
  MessageRequester(_Title, _Text)
  UnlockMutex(_TMMMutex)
EndMacro


Procedure myStructCopy(Dummy)
  Protected myLocal.myStruct
  Protected i
  
  CompilerIf #LockCopy
    LockMutex(_CopyMutex)
  CompilerEndIf
  CopyStructure(@myGlob, @myLocal, myStruct)
  CompilerIf #LockCopy
    UnlockMutex(_CopyMutex)
  CompilerEndIf
  
  If MapSize(myLocal\MyMap()) <> #NB_Elements
    ThreadModalMessageBox("Error", "Element count does not match: " + Str(MapSize(myLocal\MyMap())))
    End
  EndIf
  For i = 1 To #NB_Elements
    If Not FindMapElement(myLocal\MyMap(), Str(i))
      ThreadModalMessageBox("Fehler", "Element count match, but Element is missing: " + Str(i))
      End
    EndIf
  Next
  
EndProcedure


For i = 1 To #NB_Threads
  CreateThread(@myStructCopy(), 0)
Next

Delay(5000)

Gruß, Alex

[ts-soft]

Das ganze ist doch ein sehr schwer zu findender Fehler, und speziell Anfänger von diesem Verhalten sicher verwirrt sind, den sobald jemand fragt ob er in einem Thread nun etwas schützen muss ist so ziemlich die 1. Aussage "Solange alle Thread nur vom selben lesen dann nicht, gemeinsame Schreibzugriffe schon". Und das trifft hier einfach nicht zu

Die Aussage ist ja auch schlichtweg falsch. Da sollte man vielleicht noch eine Ergänzung zur Hilfe hinzufügen.
Eine weniger anfällige CopyStructure() Methode wäre natürlich auch schön, wenn es mit geringem Aufwand möglich sein sollte (was ich eigentlich nicht glaube).

Gruß
Thomas

[cxAlex]

Natürlich kommt es immer auf den Einzelfall an was man schützen muss. Aber der Anfänger merkt es sich halt meist so, Lesen keine Mutex, schreiben Mutex, ganz grob.

Sicher, könnte man auch einfach in der Hilfe vermerken, trotzdem würde ich das ganze gern intern gesichert sehen. Vielleicht mal einen FeatureRequest posten.

Gruß, Alex

[STARGÅTE]

Da CopyStructure() nichts anderes macht als, CopyMemory, CopyArray, CopyList und CopyMap ... und der Fehler bei einer Liste in der Struktur nicht auftritt (habs probiert), liegt der Bug wohl er bei
CopyMap

Das man mit Foreach eine Map/List nicht von mehreren Orten gleichzeitig durchgehen darf, sollte klar sein.
Aber wenn es ein kompakten Befehl CopyMap gibt, sollte dort eigentlich nicht Forach benutzt werden, was hier aber scheinbar doch der fall ist, und es somit ein durcheinander gibt.

Es sollte also wenn dann als Bug in CopyMap gemeldet werden.

[cxAlex]

Scheint wirklich an der Map zu liegen, rein auf Maps umgebaut ensteht genau derselbe Fehler:

Code: Alles auswählen

EnableExplicit

#NB_Elements = 100
#NB_Threads = 100

; Switch to Test:
#LockCopy = #False

Global NewMap myGlobalMap()
Define i

For i = 1 To #NB_Elements
  MyGlobalMap(Str(i)) = i
Next

Global _TMMMutex = CreateMutex()
CompilerIf #LockCopy
  Global _CopyMutex = CreateMutex()
CompilerEndIf

Macro ThreadModalMessageBox(_Title, _Text)
  LockMutex(_TMMMutex)
  MessageRequester(_Title, _Text)
  UnlockMutex(_TMMMutex)
EndMacro


Procedure myStructCopy(Dummy)
  Protected NewMap myLocalMap()
  Protected i
  
  CompilerIf #LockCopy
    LockMutex(_CopyMutex)
  CompilerEndIf
  CopyMap(myGlobalMap(), myLocalMap())
  CompilerIf #LockCopy
    UnlockMutex(_CopyMutex)
  CompilerEndIf
  
  If MapSize(myLocalMap()) <> #NB_Elements
    ThreadModalMessageBox("Error", "Element count does not match: " + Str(MapSize(myLocalMap())))
    End
  EndIf
  For i = 1 To #NB_Elements
    If Not FindMapElement(myLocalMap(), Str(i))
      ThreadModalMessageBox("Fehler", "Element count match, but Element is missing: " + Str(i))
      End
    EndIf
  Next
  
EndProcedure


For i = 1 To #NB_Threads
  CreateThread(@myStructCopy(), 0)
Next

Delay(5000)

Gruß, Alex