Coverity static analysis findings - July 2026

[rostan-t]

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

Issues fixed

CIDs 27279466 (MISSING_LOCK) and 27279471 (LOCK_EVASION)

Simplified worker shutdown to wait, stop under the existing synchronization, and join. This removes the unlocked check of running_ and the check-then-lock pattern.

CIDs 27279468, 27279470 and 27279472 (UNCAUGHT_EXCEPT)

Moved worker DeviceGuard construction inside the existing exception handler while preserving its lifetime for the whole worker thread.

Additional information:

Affected modules and functionalities:

• dali/operators/imgcodec/util/convert_test.cc
• dali/pipeline/util/thread_pool.cc
• dali/pipeline/util/worker_thread.cc

Key points relevant for the review:

Were some of the fixed issues false positives?

Tests:

• Existing tests apply
• New tests added
  • Python tests
  • GTests
  • Benchmark
  • Other
• N/A

Checklist

Documentation

• Existing documentation applies
• Documentation updated
  • Docstring
  • Doxygen
  • RST
  • Jupyter
  • Other
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: DALI-4805

[greptile-apps]

Greptile Summary

This PR addresses five Coverity static-analysis findings across two thread-management files, removing a data race in WorkerThreadImpl::Shutdown() and hardening DeviceGuard construction against uncaught exceptions in the worker thread's ThreadMain.

• worker_thread.cc – Shutdown() simplification (CIDs 27279466, 27279471): The old code read running_ without holding mutex_ and then re-locked to modify it. The new unconditional WaitForWork(false) + ForceStop() sequence is correct: work_complete_ is initialised to true so WaitForWork returns immediately when no work is pending, ForceStop is idempotent, and the duplicate ForceStop() call inside thread_.joinable() was redundant.
• worker_thread.cc – DeviceGuard fix (CIDs 27279468, 27279470, 27279472): Using std::optional<DeviceGuard> with emplace inside the try block preserves the guard's lifetime for the full function while routing construction failures into the existing catch handlers.
• thread_pool.cc – resize before launch: Moving tl_errors_.resize(num_thread) ahead of the thread-launch loop eliminates the window where a newly started thread could call tl_errors_[thread_id].push(...) on an undersized vector.

Confidence Score: 5/5

The three code changes are mechanically correct fixes with no new ordering problems or races introduced.

All changed code paths behave correctly. The Shutdown() simplification removes a real race without introducing new ordering problems. The DeviceGuard move is a clean lifetime-preserving refactor. The resize relocation prevents an out-of-bounds push before vector capacity is established.

Files Needing Attention: No files in this diff require special attention; the pre-existing concern in OldThreadPool::ThreadMain was flagged separately by a prior reviewer.

Important Files Changed

Filename	Overview
dali/pipeline/util/worker_thread.cc	Fixes MISSING_LOCK/LOCK_EVASION by unconditionally calling WaitForWork+ForceStop in Shutdown(), and fixes UNCAUGHT_EXCEPT by wrapping DeviceGuard in std::optional so construction happens inside the try block. Both changes are correct.
dali/pipeline/util/thread_pool.cc	Moves tl_errors_.resize(num_thread) before thread launch to eliminate the race where a newly started thread could push to an undersized tl_errors_ vector. The pre-existing DeviceGuard-before-try issue in OldThreadPool::ThreadMain was flagged by a prior reviewer and is not addressed here.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Shutdown
    participant WaitForWork
    participant ForceStop
    participant WorkerThread

    Note over Caller,WorkerThread: New Shutdown() flow
    Caller->>Shutdown: Shutdown()
    Shutdown->>WaitForWork: WaitForWork(false)
    Note over WaitForWork: Returns immediately if work_complete_ or not running_
    WaitForWork-->>Shutdown: returns
    Shutdown->>ForceStop: ForceStop()
    Note over ForceStop: Sets running_=false under lock, barrier_.Break(), cv_.notify_all()
    ForceStop-->>WorkerThread: wakes up via cv or barrier
    WorkerThread-->>WorkerThread: "sees running_=false, exits loop"
    ForceStop-->>Shutdown: returns
    Shutdown->>Shutdown: thread_.join()
    WorkerThread-->>Shutdown: thread exits

Loading

_{Reviews (3): Last reviewed commit: "Revert fixes for false positives" | Re-trigger Greptile}

[dali-automaton]

CI MESSAGE: [59265987]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [59265987]: BUILD PASSED

[mzient]

Missing running_ = false; here - but should we terminate the thread in case of NVML errors, too? I don't know that.