[REGRESSION] Device APIs crash application when called after `Device::close()`

[diablodale]

Commit 4b3c687 is a regression. It causes most Device APIs to crash applications with fatal errors like SIGSEGV when Device apis are called after Device::close().

I suggest that commit be reverted.

Setup

• all platforms, os, and compilers
• Depthai-core v2.21.1 and all later versions

Repro

I found during code review. Then confirmed with the following test case.

TEST_CASE("Use Device APIs after close") {
    constexpr auto TEST_TIMEOUT = 20s;

    // Wait for 3s to acquire a device.
    auto t1 = steady_clock::now();
    vector<dai::DeviceInfo> availableDevices;
    do {
        availableDevices = dai::Device::getAllAvailableDevices();
        this_thread::sleep_for(500ms);
    } while(availableDevices.empty() && steady_clock::now() - t1 <= 3s);
    if(availableDevices.empty()) throw std::runtime_error("No devices found");

    // Create pipeline
    dai::Pipeline pipeline;

    // Define color source and output
    auto camColor = pipeline.create<dai::node::ColorCamera>();
    auto xoutColor = pipeline.create<dai::node::XLinkOut>();
    xoutColor->setStreamName("color");
    camColor->video.link(xoutColor->input);

    // Color properties
    camColor->setBoardSocket(dai::CameraBoardSocket::RGB);
    camColor->setResolution(dai::ColorCameraProperties::SensorResolution::THE_1080_P);
    camColor->setFps(30.0f);

    // Create device
    auto device = make_shared<dai::Device>(pipeline, availableDevices[0], dai::UsbSpeed::SUPER);
    cout << "MXID: " << device->getMxId() << "\nConnected cameras: ";
    for(const auto& cam : device->getConnectedCameras()) {
        cout << cam << " ";
    }
    cout << endl;

    // Get color queue and wait for one frame
    bool ignore{};
    if(device->getOutputQueue("color")->get<dai::ImgFrame>(TEST_TIMEOUT, ignore)) {
        cout << "Device " << device->getMxId() << " message arrived\n";
    }

    // Close device
    device->close();

    // Validate Device API behaviors
    CHECK_THROWS(std::ignore = device->getMxId());
    CHECK_THROWS(device->setXLinkChunkSize(1024));
    CHECK_NOTHROW(std::ignore = device->getDeviceInfo().name);
    CHECK_NOTHROW(device->setLogOutputLevel(dai::LogLevel::WARN));
    CHECK_NOTHROW(std::ignore = device->readCalibration());
}

Results

The app crashes and/or fails multiple catch2 rules

Expected

No crash. Test case passes.

Workaround

None. 4b3c687 was hoped to fix another bug. But instead, it creates a larger bug for a larger customer audience.

[diablodale]

Ack. Found another threading/crash bug on this same code which I think better to write here instead of in separate issue.

Image two threads. And there is no predicting which thread is first, last, fastest, slowest, when each or both are paused in the middle, etc.

Thread 1

Calls a Device api which cascades to a call to the RPC lambda pimpl->rpcClient.

Thread 2

Calls Device::close(), Device destructor, etc. This function has code...

depthai-core/src/device/DeviceBase.cpp

Lines 429 to 430 in 125feb8

	pimpl->rpcStream = nullptr;
	pimpl->rpcClient = nullptr;

💣Nothing good. Maybe errors. Maybe a crash. Unpredictable.
Thread 2 sets pimpl->rpcClient = nullptr (which destructs the RPC lambda and all its resources) while thread 1 is inside that very same RPC lambda being destructed.

[diablodale]

code Luxonis added after v2.22 into DeviceBase::closeImpl starting at...

depthai-core/src/device/DeviceBase.cpp

Lines 594 to 604 in eba7400

	if(!dumpOnly) {
	auto timeout = getCrashdumpTimeout(deviceInfo.protocol);
	// Get crash dump if needed
	if(shouldGetCrashDump && timeout > 0) {
	pimpl->logger.debug("Getting crash dump...");
	auto t1 = steady_clock::now();
	bool gotDump = false;
	bool found = false;
	do {
	DeviceInfo rebootingDeviceInfo;
	std::tie(found, rebootingDeviceInfo) = XLinkConnection::getDeviceByMxId(deviceInfo.getMxId(), X_LINK_ANY_STATE, false);

further conflicts with crashes and thread safety are described and fixed in my fork above. Luxonis additional code further increases chances of crashes