gabr42 / gabr42/OmniThreadLibrary

Hang due to incorrect exception handling

Open
#213 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Pascal
Stars
513
Forks
147
PR merge metrics
No merged PRs in 30d

Description

Hi!

Our team has stumbled upon tricky issue in the OTL, resulting in permanent service hang.

Unfortunately, I do not have minimal example for reproducing the issue (though I can construct one if needed), but I'll try to describe problem in detailed enough manner.

The code enough to reproduce problem is:

```
Loop := Parallel.ForEach...NoWait;
Loop.Execute;
```
and add `raise.Exception.Create` in the `DsiAllocateHwnd`;

`.Execute`, after going through several layers, occurs in the `TOmniParallelLoopBase.InternalExecuteTask`, that initializes loop.
One of the steps is calling task.Unobserved, just before starting the loop process.
`Unobserved `results in creation of monitor, which needs a handle, that is allocated in `DsiAllocateHwnd`.

The trick is that `DsiAllocateHwnd `fails to create window for some reason (didn't figure out why yet, probably, handles/atom leak, will figure out later). As a result, it raises an exception. Hence, it is an exception in the constructor; this results in destroying of the object. Destructor tries to access `emMonitoredTasks `that hasn't yet been initialized, that results in AV while handling an exception, which in its turn complicates debugging.
```
constructor TOmniEventMonitor.Create(AOwner: TComponent);
begin
inherited;
{$IFDEF MSWINDOWS}
emMessageWindow := DSiAllocateHWnd(WndProc);
Win32Check(emMessageWindow <> 0);
{$ENDIF}
emMonitoredTasks := CreateInterfaceDictionary;
emMonitoredPools := CreateInterfaceDictionary;
emThreadID := {$IFDEF MSWINDOWS}GetCurrentThreadID{$ELSE}TThread.CurrentThread.ThreadID{$ENDIF};
end; { TOmniEventMonitor.Create }

destructor TOmniEventMonitor.Destroy;
var
intfKV : TOmniInterfaceDictionaryPair;
{$IFDEF MSWINDOWS}
winHandle: THandle;
{$ENDIF}
begin
for intfKV in emMonitoredTasks do
(intfKV.Value as IOmniTaskControl).RemoveMonitor;
```

After all, exception gets propagated to the upper level, where the loop has been created; as a result, the interface gets released by hidden finally section, calling destructor:
```
destructor TOmniParallelLoopBase.Destroy;
begin
if assigned(FCountStopped) then
{$IFDEF MSWINDOWS}
WaitForSingleObject(FCountStopped.Handle, INFINITE);
```

And here it sleeps forever, since `FCountStopped `never gets down to 0, since loop hasn't been started. Also, exception that has been raised, never gets to a handler/logger.

For myself, I'm going to add ad-hoc logging to DsiAllocateHwnd to figure out when and why it happens and wrap .Execute in the additional try..except block for logging, though it'd be great to fix initial issue with destructor been stuck in the infinite wait - if cycle hasn't been started, there's no need to wait for it to finish when destroying. Also, exception in the monitor destructor is annoying because it is masking original one. Probably, it'd be enough to check if monitor was properly initialized (maybe emMonitoredTasks been assigned at all) before trying to access its fields.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.