HotXLS File Monitor-callback deadlock in Delphi

HotXLS bewaakt een workbook op schijf met TXLSFileMonitor en vuurt OnFileChanged af wanneer een ander proces over het bestand heen opslaat. Monitor.Active := False zetten vanuit die handler bleef vroeger voor eeuwig hangen, omdat de worker thread nog steeds stond te wachten op de callback die hij net had afgeleverd. De fix is om het thread object de identiteit te laten dragen van de callback die op dat moment draait

Waarom hangt het stoppen van een file monitor binnen zijn eigen handler?

Omdat de deactiveringscall en de callback op twee verschillende threads draaien, en elk van beide wacht tot de ander klaar is. TXLSFileMonitor wordt geleverd met SynchronizeEvent op True, wat betekent dat de polling thread je handler niet zelf aanroept. Hij roept TThread.Synchronize aan, geeft de methode aan de main thread, en blokkeert dan tot de main thread hem heeft uitgevoerd en voltooiing heeft gesignaleerd. Dat is precies wat je wilt voor een handler die een grid herlaadt of een form opnieuw tekent, en het is ook wat de voor de hand liggende deactiveringscall tot een valstrik maakt

procedure TMainForm.HandleFileChanged(Sender: TObject);
begin
  // We zijn hier op de main thread, dankzij TThread.Synchronize
  FBook.LoadFromFile(FMonitor.FileName);
  RefreshGrid;
  // De gebruiker wilde maar één herlading, dus stop met bewaken
  FMonitor.Active := False;   // keerde vroeger nooit terug
end;

Volg de twee stacks en de cyclus is meteen duidelijk. De main thread zit in SetActive(False), die de worker beëindigt en daarna Worker.WaitFor aanroept om zeker te zijn dat de thread echt is afgesloten voordat het object wordt vrijgemaakt. De worker thread staat ondertussen geparkeerd in TThread.Synchronize, wachtend tot de main thread terugkeert uit precies die handler die SetActive aanroept. Geen van beide kanten kan bewegen. De applicatie crasht niet, gooit geen exception, ze stopt gewoon met tekenen, wat de slechtste diagnostische handtekening is die een bug kan hebben

Dezelfde deadlock heeft een tweede gezicht

Synchronisatie uitzetten vermijdt het probleem niet, het verplaatst het alleen. Met SynchronizeEvent op False — de verstandige keuze in een service of een console tool zonder message loop — roept TXLSFileMonitor de handler direct op de polling thread aan. Je handler draait nu op de worker, dus een call naar Monitor.Active := False van binnenuit bereikt Worker.WaitFor op de worker thread zelf. Een thread kan niet op zijn eigen afsluiting wachten, dus blokkeert hij tot iets van buiten het proces beëindigt. Beide varianten zijn dezelfde bug: SetActive(False) ging ervan uit dat hij altijd van buiten de callback werd aangeroepen. Die aanname geldt voor een Close-knop of een form destructor, en faalt bij precies de ene call site waar mensen als eerste naar grijpen, en daarom heeft hij het zo lang overleefd. Elke component die een thread bezit én gebruikerscode eruit dispatched, erft hetzelfde gevaar, en de oplossing is in elk geval hetzelfde — het teardown-pad moet kunnen zien of het vanuit een callback draait die de thread die hij op het punt staat te joinen nog vasthoudt

De worker thread de callback-identiteit laten dragen

HotXLS lost het op door op het thread object zelf vast te leggen welke OS thread op dat moment de handler uitvoert. TXLSFileMonitorThread.FireEvent stempelt FCallbackThreadID met GetCurrentThreadId vlak voordat hij de handler aanroept en wist het daarna in een finally-blok. Dat ene veld volstaat voor het deactiveringspad om onderscheid te maken tussen "aangeroepen vanuit niet-gerelateerde code" en "aangeroepen vanuit mijn eigen callback", en het werkt identiek voor beide dispatch-modi, want in het gesynchroniseerde geval rapporteert GetCurrentThreadId in FireEvent al de main thread

procedure TXLSFileMonitorThread.FireEvent;
var
  Owner: TXLSFileMonitor;
  Handler: TNotifyEvent;
begin
  // Een queued callback kan tijdens deactivering door WaitFor worden afgeleverd
  if Terminated then
    Exit;
  Owner := FOwner;
  Handler := Owner.FOnFileChanged;
  if not Assigned(Handler) then
    Exit;
  FCallbackThreadID := GetCurrentThreadId;
  try
    Handler(Owner);
  finally
    // De handler heeft Owner misschien vernietigd, maar deze thread blijft leven
    FCallbackThreadID := 0;
  end;
end;

De deactivatietak van SetActive doet dan drie dingen in een bewuste volgorde. Hij ontkoppelt de thread van de monitor door eerst FThread te wissen, zodat GetActive vanaf dat moment False rapporteert en een reentrant Active := False in dezelfde handler meteen afsluit in plaats van de thread twee keer te proberen af te breken. Hij roept Terminate aan. Pas daarna beslist hij hoe het thread object wordt teruggehaald

Worker := TXLSFileMonitorThread(FThread);
FThread := nil;            // de monitor is vanaf dit punt inactief
Worker.Terminate;
FHasBaseline := False;
if Worker.FCallbackThreadID = GetCurrentThreadId then
  // Zowel directe als gesynchroniseerde handlers moeten terugkeren voordat de worker
  // kan afsluiten; hij geeft zichzelf vrij nadat de callback is afgewikkeld
  Worker.FreeOnTerminate := True
else
begin
  Worker.WaitFor;
  Worker.Free;
end;

Als de identiteit overeenkomt, wacht HotXLS nooit. Hij zet FreeOnTerminate := True en keert terug, laat de handler afmaken, de Synchronize-call voltooien, Execute Terminated zien, en de RTL het thread object vrijmaken zodra de callback stack volledig is afgewikkeld. Als de identiteit niet overeenkomt — een Close-knop, een destructor, een achtergrondjob — is de klassieke WaitFor plus Free nog steeds het juiste, omdat je daarmee een harde garantie krijgt dat er geen verdere callback meer kan vuren nadat Active := False is teruggekeerd. Twee exit-paden, één veld om ertussen te kiezen. Eén gevolg verdient expliciete vermelding: op het self-release-pad overleeft de worker de handler. Niets in het return-pad van FireEvent mag het monitor object nog aanraken, want de handler heeft het recht gehad het vrij te maken. Daarom cached FireEvent Owner en Handler vooraf in lokale variabelen en raakt hij in de finally-clausule alleen zijn eigen veld aan. Een verdwaalde FOwner.Something na de handler-call zou een opgeloste deadlock veranderen in een use-after-free, wat een strikt slechter resultaat is

Waarom moet FireEvent Terminated checken voordat hij iets doet?

Omdat WaitFor geen passieve wait is. Op de main thread blijft de RTL-implementatie van TThread.WaitFor de synchronisatiequeue legen terwijl hij blokkeert, juist zodat een thread die in Synchronize geparkeerd staat kan voltooien en afsluiten in plaats van tegen de shutdown te deadlocken. Dat is een feature, en het betekent dat een callback die microseconden vóór je Active := False-call vanaf een Close-knop in de queue is gezet, nog steeds tijdens de teardown kan worden afgeleverd, nadat de monitor zijn FThread-referentie al heeft laten vallen. De guard bovenaan FireEvent maakt die dispatch een no-op. Zonder hem kan een applicatie die haar hoofdform sluit terwijl een save op het bewaakte bestand neerkomt, een reload-handler laten draaien tegen half-weggegooide state, en de resulterende access violation ziet er totaal niet uit als een shutdown-orderingprobleem. Als je eigen component werk via Synchronize dispatched, generaliseert de regel: elke gesynchroniseerde methode heeft een Terminated-check als eerste statement nodig, niet omdat de thread misschien weg is, maar omdat de queue de beslissing om te stoppen overleeft

Dezelfde vorm van bug in het CSV-importpad

De audit die de monitor-fix opleverde, bracht een verwant defect in de tekstimporter aan het licht, en het is de moeite waard ze naast elkaar te lezen omdat de onderliggende fout identiek is: een stuk configuratie dat wordt verbruikt alsof het scratch-state is. TXLSCSVLoader stelt een SkipRows-property beschikbaar voor de bannerregels die in geëxporteerde rapporten boven de echte header staan. De parser heeft een countdown nodig terwijl hij die regels doorwerkt, en de oorspronkelijke implementatie telde af op het backing field van de property zelf. Importeer één bestand en SkipRows werd stilzwijgend nul; gebruik dezelfde loader voor een tweede bestand en de bannerregels kwamen in de grid terecht

Loader := TXLSCSVLoader.Create;
try
  Loader.SkipRows := 2;             // twee bannerregels boven de header
  Loader.ParseFile('jan-export.csv');
  ReadGrid(Loader);
  // SkipRows is hier nog steeds 2: FSkipRemaining wordt per parse gereset
  Loader.ParseFile('feb-export.csv');
  ReadGrid(Loader);
finally
  Loader.Free;
end;

HotXLS houdt FSkipRows nu als de gedeclareerde instelling en reset een aparte FSkipRemaining-teller aan het begin van elke parse, in zowel het delimited- als het fixed-width-pad. De algemene regel is goedkoop toe te passen en betaalt zichzelf terug: als een loop erop aftelt, is het geen backing field van een property. Dezelfde batch heeft ook XlsCsvTryParseISODateTime aangescherpt, die bereid was een parsebare prefix als volledige datum te accepteren. Hij valideert nu het hele veld — lengte, separatorposities en elk cijfer — zodat een waarde zoals een orderreferentie die toevallig begint met iets dat op een datum lijkt, tekst blijft in plaats van een foutieve TDateTime te worden. Stille typecoercie bij import is downstream veel duurder om te debuggen dan een afgewezen veld, een thema dat ook doorloopt in hoe HotXLS een workbook beschermt tegen gelijktijdige lezers en schrijvers

Wat deze fix je wel en niet oplevert

Wees precies over de garantie. De thread-ownershipwijziging maakt precies één voorheen fataal patroon veilig: een TXLSFileMonitor deactiveren of vrijmaken vanuit zijn eigen OnFileChanged-handler, in beide dispatch-modi. Het is geen algemeen concurrency-ontwerp, en het maakt de component niet veilig om vanuit willekeurige threads aan te sturen

• Property writes zijn nog steeds niet thread-safe. Zet FileName, IntervalMs en SynchronizeEvent vanuit één thread, normaal gesproken degene die de monitor heeft aangemaakt
• IntervalMs geldt vanaf de volgende activering, en de worker slaapt in korte stukjes zodat Terminate snel wordt gehonoreerd, zelfs met een lange poll-periode
• Detectie gebeurt op last-write tijd plus grootte, dus een wijziging die beide gelijk houdt is onzichtbaar; een bestand dat halverwege een save verdwijnt vuurt niet, en het event arriveert zodra de nieuwe inhoud er is
• Met SynchronizeEvent op False draait je handler op de worker thread, en elke regel over het aanraken van VCL controls vanuit een background thread geldt voor alles wat je erbinnen doet
• Een workbook herladen vanuit de handler vraagt nog steeds zijn eigen discipline, vooral als andere code een referentie op het te vervangen boek vasthoudt

In de praktijk verdient de reload-handler de meeste zorg, niet de monitor. Een change event arriveert meestal terwijl het schrijfproces nog aan het settlen is, dus een load die het bestand meteen opent, kan een half-committed save tegenkomen; één keer opnieuw proberen na een korte vertraging is robuuster dan de eerste stamp vertrouwen. Als je eigen code het bewaakte bestand schrijft, geven het staged temp bestand en de atomic rename die in hoe HotXLS elke save crash-safe maakt de monitor ook een schone single-transition event om waar te nemen in plaats van een stroom van tussentijdse groottes. En als meerdere watchers dezelfde reload-routine voeden, is de read gate rond gelijktijdige ZIP-inflatie het volgende om te lezen. De bredere les overleeft de specifieke component. Elke class die een thread bezit en terugbelt naar gebruikerscode heeft een teardown-pad nodig dat zijn eigen callback frame kan herkennen, want de handler is de meest voor de hand liggende plek ter wereld voor een gebruiker om stop te zeggen. Het registreren van de callback thread id kost één DWORD en verandert een onverklaarbare hang in een gewone object-lifetime-vraag. File monitoring, atomic saves en tekstimport zitten allemaal in de HotXLS Delphi Component voor Delphi en C++Builder, zonder extra configuratie om het in te schakelen