View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002956 | CCdciel | General | public | 26-10-06 23:15 | 26-10-08 13:07 |
| Reporter | Jan Dubovsky | Assigned To | Patrick Chevalley | ||
| Priority | high | Severity | major | Reproducibility | N/A |
| Status | resolved | Resolution | no change required | ||
| Summary | 0002956: Use-after-free when closing/disconnecting the INDI watchdog or safety monitor | ||||
| Description | `T_indiwatchdog.Destroy` (src/cu_indiwatchdog.pas, ~line 129-141) and `T_indisafety.Destroy` (src/cu_indisafety.pas, ~line 120-130) only set `indiclient.onServerDisconnected := nil` and free their own timers. They never call `indiclient.Terminate` and never wait for the INDI client thread to stop. `indiclient` is a `TIndiBaseClient` (`TThread` with `FreeOnTerminate := True`, see component/indiclient/indibaseclient.pas:229) whose other callbacks (`onNewDevice`, `onNewProperty`, `onNewSwitch`, `onNewMessage`, etc.) are still bound to the watchdog/safety instance being destroyed. `DestroyDevices` in src/pu_main.pas (~line 2497-2509) calls `watchdog.Free; safety.Free;` with no prior `Disconnect` call. If the device is still connected when the app closes (or whenever the monitor is destroyed while connected), the INDI client thread keeps running in the background and will invoke a method on the now-freed watchdog/safety object the next time it receives a message from the INDI server — a classic use-after-free. | ||||
| Steps To Reproduce | 1. Connect the Watchdog or Safety monitor panel to a live INDI device. 2. Close CCDciel (or otherwise trigger device teardown) without first disconnecting that device. 3. If the INDI server sends another property/message update in the small window before the process exits, the freed object is accessed — intermittent crash on shutdown/disconnect. | ||||
| Additional Information | ### Suggested fix In `Destroy`, call `indiclient.Terminate` (and ideally wait for it, or at least nil out every callback the thread can invoke) before freeing the owner object — mirroring what `Disconnect` already does elsewhere, but `Disconnect` is never called from `DestroyDevices`. | ||||
| Tags | crash | ||||
|
|
This is a false detection because the disconnection is always done before to destroy the objects. When exiting the program the disconnection is done early in onCloseQuery event, long before onDestroy. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 26-10-06 23:15 | Jan Dubovsky | New Issue | |
| 26-10-06 23:15 | Jan Dubovsky | Tag Attached: crash | |
| 26-10-08 13:07 | Patrick Chevalley | Assigned To | => Patrick Chevalley |
| 26-10-08 13:07 | Patrick Chevalley | Status | new => resolved |
| 26-10-08 13:07 | Patrick Chevalley | Resolution | open => no change required |
| 26-10-08 13:07 | Patrick Chevalley | Note Added: 0009973 |
