2010-03-25 13 views
14

J'ai parfois un problème avec un blocage en détruisant certains threads. J'ai essayé de déboguer le problème mais le blocage ne semble jamais exister lors du débogage dans l'EDI, peut-être à cause de la faible vitesse des événements dans l'EDI.Delphi threads deadlock

Le problème:

Le thread principal crée plusieurs threads lorsque l'application démarre. Les threads sont toujours actifs et synchronisés avec le thread principal. Aucun problème du tout. Les threads sont détruits lorsque l'application se termine (mainform.onclose) comme ceci:

thread1.terminate; 
thread1.waitfor; 
thread1.free; 

et ainsi de suite.

Mais parfois l'un des threads (qui enregistre une chaîne dans un mémo, en utilisant la synchronisation) verrouillera toute l'application lors de la fermeture. Je soupçonne que le thread se synchronise quand j'appelle waitform et que l'harmaggeddon se produit, mais c'est juste une supposition car le blocage ne se produit jamais lors du débogage (ou je n'ai jamais pu le reproduire quand même). Aucun conseil?

+1

Code serait utile, au moins avec votre appel à synchroniser et nettoyer le thread incriminé. –

Répondre

27

La journalisation des messages est juste l'un de ces domaines où Synchronize() n'a aucun sens du tout. Vous devriez plutôt créer un objet cible de journal, qui a une liste de chaînes, protégé par une section critique, et y ajouter vos messages de journal. Le thread VCL principal supprime les messages du journal de cette liste et les affiche dans la fenêtre du journal. Cela présente plusieurs avantages:

  • Vous n'avez pas besoin d'appeler Synchronize(), qui est juste une mauvaise idée. Un bon effet secondaire est que vos problèmes d'arrêt disparaissent.

  • Les threads de travail peuvent continuer leur travail sans bloquer la gestion des événements de thread principal ou sur d'autres threads essayant de consigner un message.

  • Les performances augmentent, car plusieurs messages peuvent être ajoutés à la fenêtre de journal en une fois. Si vous utilisez BeginUpdate() et EndUpdate() cela accélérera les choses.

Il n'y a pas d'inconvénients que je peux voir - l'ordre des messages de journal est également préservé.

Edit:

Je vais ajouter un peu plus d'informations et un peu de code pour jouer avec, afin d'illustrer qu'il ya de bien meilleures façons de faire ce que vous devez faire. L'appel Synchronize() à partir d'un thread différent de celui du thread d'application principal dans un programme VCL provoquera le blocage du thread appelant, l'exécution du code passé dans le contexte du thread VCL, puis le thread appelant sera débloqué et continuer à courir. Cela a peut-être été une bonne idée à l'époque des machines monoprocesseur, sur lesquelles un seul thread peut fonctionner à la fois, mais avec plusieurs processeurs ou noyaux c'est un gâchis géant et devrait être évité à tout prix. Si vous avez 8 threads de travail sur une machine à 8 cœurs, les appeler Synchronize() limitera probablement le débit à une fraction de ce qui est possible.

En fait, appeler Synchronize() n'a jamais été une bonne idée, car il peut conduire à des impasses. Une raison de plus convaincante de ne pas l'utiliser, jamais.

Utiliser PostMessage() pour envoyer les messages du journal se chargeront de la question de l'impasse, mais il a ses propres problèmes:

  • Chaque chaîne de journal PROVOQUERONT un message à être affiché et traité, causant beaucoup de frais généraux. Il n'y a aucun moyen de gérer plusieurs messages de journaux en une fois.

  • messages Windows ne peuvent transporter des données de taille mot-machine dans les paramètres. L'envoi de chaînes est donc impossible. Envoyer des chaînes après un transtypage à PChar est dangereux car la chaîne peut avoir été libérée au moment où le message est traité. Allouer de la mémoire dans le thread de travail et libérer cette mémoire dans le thread VCL après que le message a été traité est une issue. Un moyen qui ajoute encore plus de frais généraux.

  • Les files d'attente de messages dans Windows ont une taille finie. La publication d'un trop grand nombre de messages peut entraîner la saturation de la file d'attente et la suppression des messages. Ce n'est pas une bonne chose, et avec le point précédent cela conduit à des fuites de mémoire.

  • Tous les messages dans la file d'attente sont traitées avant que des messages de temporisation ou de peinture seront générés. Un flux continu de nombreux messages postés peut donc provoquer l'arrêt du programme.

Une structure de données qui collecte les messages de journalisation pourrait ressembler à ceci:

type 
    TLogTarget = class(TObject) 
    private 
    fCritSect: TCriticalSection; 
    fMsgs: TStrings; 
    public 
    constructor Create; 
    destructor Destroy; override; 

    procedure GetLoggedMsgs(AMsgs: TStrings); 
    procedure LogMessage(const AMsg: string); 
    end; 

constructor TLogTarget.Create; 
begin 
    inherited; 
    fCritSect := TCriticalSection.Create; 
    fMsgs := TStringList.Create; 
end; 

destructor TLogTarget.Destroy; 
begin 
    fMsgs.Free; 
    fCritSect.Free; 
    inherited; 
end; 

procedure TLogTarget.GetLoggedMsgs(AMsgs: TStrings); 
begin 
    if AMsgs <> nil then begin 
    fCritSect.Enter; 
    try 
     AMsgs.Assign(fMsgs); 
     fMsgs.Clear; 
    finally 
     fCritSect.Leave; 
    end; 
    end; 
end; 

procedure TLogTarget.LogMessage(const AMsg: string); 
begin 
    fCritSect.Enter; 
    try 
    fMsgs.Add(AMsg); 
    finally 
    fCritSect.Leave; 
    end; 
end; 

De nombreux threads peuvent appeler LogMessage() en même temps, entrer dans la section critique sérialisera accès à la liste, et après avoir ajouté leur message la les discussions peuvent continuer avec leur travail.

Cela laisse la question de savoir comment le thread VCL sait quand appeler GetLoggedMsgs() pour supprimer les messages de l'objet et les ajouter à la fenêtre. La version d'un pauvre serait d'avoir une minuterie et un sondage. Une meilleure façon serait d'appeler PostMessage() lorsqu'un message de journal est ajouté:

procedure TLogTarget.LogMessage(const AMsg: string); 
begin 
    fCritSect.Enter; 
    try 
    fMsgs.Add(AMsg); 
    PostMessage(fNotificationHandle, WM_USER, 0, 0); 
    finally 
    fCritSect.Leave; 
    end; 
end; 

Cela a toujours le problème avec trop de messages postés.Un message doit être affiché que lorsque le précédent a été traité:

procedure TLogTarget.LogMessage(const AMsg: string); 
begin 
    fCritSect.Enter; 
    try 
    fMsgs.Add(AMsg); 
    if InterlockedExchange(fMessagePosted, 1) = 0 then 
     PostMessage(fNotificationHandle, WM_USER, 0, 0); 
    finally 
    fCritSect.Leave; 
    end; 
end; 

qui peut encore être améliorée, cependant. L'utilisation d'une minuterie résout le problème des messages postés remplissant la file d'attente. Ce qui suit est une petite classe qui implémente ceci:

type 
    TMainThreadNotification = class(TObject) 
    private 
    fNotificationMsg: Cardinal; 
    fNotificationRequest: integer; 
    fNotificationWnd: HWND; 
    fOnNotify: TNotifyEvent; 
    procedure DoNotify; 
    procedure NotificationWndMethod(var AMsg: TMessage); 
    public 
    constructor Create; 
    destructor Destroy; override; 

    procedure RequestNotification; 
    public 
    property OnNotify: TNotifyEvent read fOnNotify write fOnNotify; 
    end; 

constructor TMainThreadNotification.Create; 
begin 
    inherited Create; 
    fNotificationMsg := RegisterWindowMessage('thrd_notification_msg'); 
    fNotificationRequest := -1; 
    fNotificationWnd := AllocateHWnd(NotificationWndMethod); 
end; 

destructor TMainThreadNotification.Destroy; 
begin 
    if IsWindow(fNotificationWnd) then 
    DeallocateHWnd(fNotificationWnd); 
    inherited Destroy; 
end; 

procedure TMainThreadNotification.DoNotify; 
begin 
    if Assigned(fOnNotify) then 
    fOnNotify(Self); 
end; 

procedure TMainThreadNotification.NotificationWndMethod(var AMsg: TMessage); 
begin 
    if AMsg.Msg = fNotificationMsg then begin 
    SetTimer(fNotificationWnd, 42, 10, nil); 
    // set to 0, so no new message will be posted 
    InterlockedExchange(fNotificationRequest, 0); 
    DoNotify; 
    AMsg.Result := 1; 
    end else if AMsg.Msg = WM_TIMER then begin 
    if InterlockedExchange(fNotificationRequest, 0) = 0 then begin 
     // set to -1, so new message can be posted 
     InterlockedExchange(fNotificationRequest, -1); 
     // and kill timer 
     KillTimer(fNotificationWnd, 42); 
    end else begin 
     // new notifications have been requested - keep timer enabled 
     DoNotify; 
    end; 
    AMsg.Result := 1; 
    end else begin 
    with AMsg do 
     Result := DefWindowProc(fNotificationWnd, Msg, WParam, LParam); 
    end; 
end; 

procedure TMainThreadNotification.RequestNotification; 
begin 
    if IsWindow(fNotificationWnd) then begin 
    if InterlockedIncrement(fNotificationRequest) = 0 then 
    PostMessage(fNotificationWnd, fNotificationMsg, 0, 0); 
    end; 
end; 

Une instance de la classe peut être ajouté à TLogTarget, pour appeler un événement de notification dans le thread principal, mais au plus quelques dizaines de fois par seconde.

+1

+ NUM_ALLOWED_VOTES :) c'est une bonne écriture! – jpfollenius

+0

Quand dois-je appeler 'RequestNotification'? – EProgrammerNotFound

+0

@MatheusFreitas: Chaque fois qu'un thread qui n'est pas le thread principal a besoin du thread principal pour faire quelque chose dans son contexte. Dans le cas du système de journalisation, un message de journal ajouté dans un thread de travail doit être affiché dans l'interface graphique. Si le thread de travail appelle 'RequestNotification()', il peut alors continuer son travail tout en étant certain que dans le futur, le thread graphique sera averti du nouveau message de journal et l'affichera. J'espère que c'est assez clair? – mghie

2

Ajouter un objet mutex au thread principal. Obtenez mutex lorsque vous essayez de fermer le formulaire. Dans un autre thread, vérifiez le mutex avant de le synchroniser dans la séquence de traitement.

7

Envisagez de remplacer Synchronize par un appel au PostMessage et de gérer ce message dans le formulaire pour ajouter un message de journal au mémo. Quelque chose le long des lignes de: (prendre comme pseudo-code)

WM_LOG = WM_USER + 1; 
... 
MyForm = class (TForm) 
    procedure LogHandler (var Msg : Tmessage); message WM_LOG; 
end; 
... 
PostMessage (Application.MainForm.Handle, WM_LOG, 0, PChar (LogStr)); 

Cela évite tous les problèmes de blocage de deux threads en attente pour l'autre. (Merci à Serg pour l'indice): Notez que passer la chaîne de la manière décrite n'est pas sûr car la chaîne peut être détruite avant que le thread VCL ne l'utilise. Comme je l'ai mentionné - c'était seulement destiné à être pseudocode.

+1

Incorrect. 'SendMessage()' bloquera juste la même chose si la boucle de message du thread de réception ne va pas gérer le message, comme quand il est dans WaitForSingleObject() ', en attendant que le thread se termine. – mghie

+1

Vous devez utiliser PostMessage au lieu de SendMessage pour éviter le blocage des threads. – kludg

+0

Bien, j'ai confondu les deux. Merci @Serg pour la critique plus constructive. – jpfollenius

1

Il est simple:

TMyThread = class(TThread) 
protected 
    FIsIdle: boolean; 
    procedure Execute; override; 
    procedure MyMethod; 
public 
    property IsIdle : boolean read FIsIdle write FIsIdle; //you should use critical section to read/write it 
end; 

procedure TMyThread.Execute; 
begin 
    try 
    while not Terminated do 
    begin 
     Synchronize(MyMethod); 
     Sleep(100); 
    end; 
    finally 
    IsIdle := true; 
    end; 
end; 

//thread destroy; 
lMyThread.Terminate; 
while not lMyThread.IsIdle do 
begin 
    CheckSynchronize; 
    Sleep(50); 
end; 
+1

Il n'est pas nécessaire d'utiliser des sections critiques pour un booléen écrit une seule fois. – mghie

0

objet TThread de Delphi (et classes héritant) appelle déjà WaitFor lors de la destruction, mais cela dépend si vous avez créé le fil avec CreateSuspended ou non. Si vous utilisez CreateSuspended = true pour effectuer une initialisation supplémentaire avant d'appeler le premier CV, vous devriez envisager de créer votre propre constructeur (en appelant le inherited Create(false);) qui effectue l'initialisation supplémentaire.