Discussion :

[Andnotor]

Bonjour à tous !

Lorsque dans un thread (plusieurs instances) on a un code comme celui-ci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
Synchronize(procedure
            begin
              inc(SyncCount);
              // SyncCount = 1 ?
              Dec(SyncCount);
            end);

on s'attend à ce que SyncCount soit toujours à 1, que l'appel en cours se termine avant la prochaine synchronisation.

Sauf que si dans la procédure synchronisée on procède à la destruction d'un TThread, ce dernier attend la fin effective de sa tâche par un WaitFor qui relance un CheckSynchronize !
Résultat, un deuxième thread peut être autorisé à accéder au principal alors que la synchronisation précédente n'est pas terminée (TMonitor mal placé/utilisé ?) !

Voici une démo minimale pour illustrer cela. Ne la laissez pas tourner pendant des heures, le nombre de threads augmentant rapidement. Ctr+F2 pour aborter.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
unit Unit1;
 
interface
 
uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls;
 
type
  TWorkerThread = class(TThread)
  protected
    procedure Execute; override;
  end;
 
  TSyncThread = class(TThread)
  protected
    procedure Execute; override;
  end;
 
  TForm1 = class(TForm)
    Label1: TLabel;
    procedure FormCreate(Sender: TObject);
  public
    WorkerThread :TWorkerThread;
  end;
 
var
  Form1: TForm1;
 
implementation
 
{$R *.dfm}
 
var SyncCount :integer;
 
{ TWorkerThread }
 
procedure TWorkerThread.Execute;
begin
  while not Terminated do
    Sleep(1000);
end;
 
{ TSyncThread }
 
procedure TSyncThread.Execute;
begin
  while not Terminated do
    Synchronize(procedure
                begin
                  inc(SyncCount);
 
                  try
                    Form1.Label1.Caption := 'SyncCount = ' +SyncCount.ToString;
                    Form1.Label1.Update;
 
                    FreeAndNil(Form1.WorkerThread);
                    Form1.WorkerThread := TWorkerThread.Create;
 
                  finally
                    Dec(SyncCount);
                  end;
                end);
end;
 
procedure TForm1.FormCreate(Sender: TObject);
begin
  WorkerThread := TWorkerThread.Create;
  TSyncThread.Create;
  TSyncThread.Create;
  TSyncThread.Create;
  TSyncThread.Create;
end;
 
end.

Pour retrouver le comportement logique, il faut englober Synchronize dans une section critique

Je suis en 10.4.2, est-ce que quelque sur un Delphi plus ancien observe le même comportement ou est-ce une régression ?

[Paul TOTH]

tient, c'est un cas de figure intéressant.

alors c'est "threadsafe" dans le sens, ou tu es dans un cas d'appel récursif ... bien qu'involontaire.

tu pourrais par exemple tester la valeur de SyncCount sans avoir besoin d'une section critique, if Syncount > 0 then Exit, elle ne risque pas de changer vu que seul le thread principal y touche, mais en effet, quand tu détruits un Thread il y a appel à CheckSynchronize qui peut lancer le Synchronize d'un autre thread, et qui peut aboutir sur la même procédure.

c'est un peu comme l'appel à Application.ProcessMessages, tu peux recliquer sur le bouton qui fait appel à cette méthode.

Note que si tu mets une section critique tu vas tomber sur un deadlock

[Andnotor]

SyncCount est juste pour la démo à l'instar de la recréation du WorkerThread. Dans l'absolu, il s'agit de traiter des demandes par ordre d'arrivée (dont certaines peuvent entraîner une destruction) et il n'est pas question dans louper une.

Oui c'est un appel récursif mais qui en soit ne poserait pas de problème s'il n'y avait qu'une SyncList unique. Mais puisque CheckSynchronize la fige et que chaque appel travaille sur la sienne, ça devient problématique.

Note que si tu mets une section critique tu vas tomber sur un deadlock

A mettre hors Synchronize évidemment

[Paul TOTH]

je n'ai sais pas comment est gérée ton affaire, mais normalement les Workers traitent des demandes, ou se mette en attente de demandes quand il n'y en a plus...mais il n'y a pas lieu d'en créer/supprimer des Workers en cours de route.

[pprem]

Oulà, cas typique qui pose problème effectivement, la solution serait dans la suppression différée ou l'auto suppression.

Le seul truc que nous assure le synchronize() c'est qu'on soit dans le thread principal au moment de son exécution, donc en cas normal un synchronize est le seul à s'exécuter et ils sont censés servir à manipuler les éléments de l'interface.

Ici je vois plusieurs solutions :
- faire un lock avant le synchronize, mais ce serait dommage
- gérer une liste des threads à supprimer, qui pourrait être traitée par un autre thread
- demander au worker de se supprimer plutôt que le supprimer dans l'autre (appel de terminale ou plus simplement changement de son tag qui pourrait être testé dans la boucle)

[Andnotor]

C'est du pilotage à distance. Les commandes arrivent par NamedPipe (le TSyncThread de mon exemple) et peuvent créer des éléments graphiques intégrant eux-mêmes leur propre tâche de mise à jour (le TWorkerThread). La suppression d'éléments entraîne logiquement la destruction de leur tâche.

Mais ces éléments peuvent également n'être que des conteneurs remplis d'autres éléments, d'où le besoin de traiter les demandes dans l'ordre d'arrivée.

Je vais super-synchroniser les opérations destructives.

Merci à vous !

[sergio_is_back]

- gérer une liste des threads à supprimer, qui pourrait être traitée par un autre thread

C'est généralement comme ça que je procède...

Soit les thread terminés, ferment toutes leurs ressources et montent un flag à True pour demander leur libération qui sera effectuée en décalé
Soit, quand je le peux, je les utilisent avec FreeOnTerminate:=True (dans le cas d'un Thread "FireAndForget")

[ShaiLeTroll]

- gérer une liste des threads à supprimer, qui pourrait être traitée par un autre thread

dernièrement j'ai aussi utilisé une List Garbage, c'est un ObjetList dans lequel je colle les objets à détruire de façon asynchrone, et c'est fait dans l'évènement Application.OnIdle avec un Garbage.Clear

C'est effectivement une des approches que j'utilise sauf que j'utilise une TThreadList comme mentionnée ci-dessus.

D'ailleurs, le Release qui existait encore dans Seattle permettant de mettre un objet dans le Purgatoire en vue d'une libération différée, par quoi est-il remplacé en Sydney puisque Déprécié

Et l'on a déjà dit tout ça que le suicide d'objet était à faire avec précaution

[ShaiLeTroll]

InterlockedIncrement et tu n'auras plus de soucis pour tes compteurs, selon mes tests d'Avril pour justement remplacer des CS, c'est 70x fois plus rapide.
ça c'est si tu veux gérer un compteur entre plusieurs threads en dehors de ta démo, hors de Synchronize, hors de de CS mais je suppose que tes compteurs c'est juste pour montrer la ré-entrance, ton souhaite réel est de protéger ton Label1 ou de protéger une Thread Factory ?

Et Synchronize, si je peux l'éviter celui-là je préfére, même si je dois passer par des variables tampons, des TThreadList et OnIdle, pour justement que le temps passé dans Synchronize ne bloque pas les threads qui l'utiliserait trop massivement.
En gros je communique avec le MainThread qui si il était un thread lambda


Quel est ton besoin réel ?
C'est recréer des Threads ?

J'utilise pour ma part Terminated et un TEvent + Compteur en InterlockedIncrement
J'ai un moteur qui lance des threads, il augmente le compteur à la création, puis les threads à leur fin décrémente le compteur, le TEvent permet que le lanceur de threads puisse attendre avec un TimeOut + un Réveil forcé.