NIDAQ: Fix bug that caused the NIDAQ adapter to crash when running the MDA wizard.

[nicost]

thread::join threw system_error on a thread that was never activated(). These threads were allocated but never starteunless using AI monitoring or tracing, so a normal shutdown joined two never started thtrads and the throw killed the JVM (or more precisely, the MM would dispapear even though the JVM was still running). Each thread class now carries a started_ flag. Join() now only joins when there is something to join.

[Copilot]

🟡 Not ready to approve

There is a shutdown-ordering/thread-lifecycle issue that can still lead to a crash (and one resource-leak-on-error-path concern) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR addresses a shutdown-time crash in the NIDAQ adapter by making monitoring-thread teardown safe when threads were allocated but never activated, and by centralizing stop/join/delete patterns to avoid unsafe wait() usage.

Changes:

• Add started_ tracking plus Join() helpers to monitoring threads to avoid std::system_error when joining never-activated threads.
• Add NIDAQHub::DestroyMonitoringThread(...) helpers and use them across shutdown/start/stop paths to standardize safe cleanup.
• Fix a DO hub port-width selection bug (uInt16 case) and correct DO hub deletion logic to avoid leaks.

File summaries

File	Description
DeviceAdapters/NIDAQ/NIDAQ.h	Adds Join()/started_ plumbing and declares hub helpers for safe monitoring-thread destruction.
DeviceAdapters/NIDAQ/NIDAQ.cpp	Implements safe destroy/join helpers, wraps shutdown to prevent exception escape, updates teardown call sites, and fixes DO hub width/deletion issues.

Review details

Suppressed comments (2)

DeviceAdapters/NIDAQ/NIDAQ.cpp:1854

• TraceMonitoringThread::Start() has multiple early returns on DAQmx* failures after creating/starting aiTask_, but does not consistently clear the task before returning. This can leak DAQmx task resources (and potentially leave a started task running) when initialization fails; consider refactoring to a single cleanup path (scope guard / goto fail) that DAQmxClearTask()s aiTask_ on every failure after creation.

    activate();
    // Only after activate() returns without throwing is there a thread to join.
    started_ = true;
    return DEVICE_OK;

DeviceAdapters/NIDAQ/NIDAQ.cpp:1135

• FinishTrace() later calls mThread_->Start() without ensuring mThread_ is non-null and not already running. This can crash if shutdown has already nulled mThread_, and it can also attempt to re-activate an already-running monitoring thread (e.g. if expected-min/max handlers restarted it during tracing). Recreate (or at least stop/join) mThread_ before starting it.

// Called from TraceMonitoringThread::svc(), i.e. on the trace thread itself.
// It must therefore never stop or join tThread_: doing so would make the trace
// thread join itself and deadlock. Only mThread_ may be touched here.
int NIDAQHub::FinishTrace()

• Files reviewed: 2/2 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[nicost]

Every early return after DAQmxCreateTask succeeded left the task created and orphaned: one path in InputMonitoringThread::Start(), six in TraceMonitoringThread::Start(). Added a private ClearTask() per class, called on each. DAQmxClearTask also stops a running task, so it's correct after DAQmxStartTask too. On DAQmxCreateTask failure itself I null the handle without clearing — DAQmx leaves it unspecified, so clearing it would be the bug.