Skip to content

PhotoVideoControl: Fix null pointer errors on camera initialization #13302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Eureak-zon
Copy link

Add comprehensive null checks for _camera and _cameraManager to prevent
TypeError exceptions during component initialization. Guard all property
bindings and method calls with null checks and provide appropriate
fallback values.

Add comprehensive null checks for _camera and _cameraManager to prevent
TypeError exceptions during component initialization. Guard all property
bindings and method calls with null checks and provide appropriate
fallback values.
@Eureak-zon Eureak-zon marked this pull request as ready for review August 14, 2025 07:51
@HTRamsey
Copy link
Collaborator

@DonLakeFlyer should this whole element just be under a loader enabled by an existing camera?

@HTRamsey HTRamsey requested a review from DonLakeFlyer August 14, 2025 16:34
@DonLakeFlyer
Copy link
Contributor

Ah, thanks. Been meaning to get to that myself. It's been making me a bit crazy.

@DonLakeFlyer
Copy link
Contributor

I need to noodle on this a bit. If you look at FlyViewTopRightColumnLayout.qml there is Loader code in there meant to prevent these null refs from happening. I think it used to work, but now it doesn't. Want to understand why first.

@DonLakeFlyer
Copy link
Contributor

So the reason the Loader isn't working is because of this:

void Vehicle::prepareDelete()
{
    // Clean up camera manager to stop all timers and prevent crashes during destruction
    if(_cameraManager) {
        // because of _cameraManager QML bindings check for nullptr won't work in the binding pipeline
        // the dangling pointer access will cause a runtime fault
        auto tmpCameras = _cameraManager;
        _cameraManager = nullptr;
        delete tmpCameras;
        emit cameraManagerChanged();
        // Note: Removed qApp->processEvents() to prevent MAVLink crashes during destruction
    }
}

If still thinking about whether there is a way to deal with this without needing to put null checks everywhere which just makes the code grungy.

@DonLakeFlyer
Copy link
Contributor

@julianoes Hey Julian, do you have any idea why Qml binding would be causing crashed in CameraManager code. If I remove this prepareDelete things seems to work fine for me and we no longer need nulls checked everywhere since the Loader which controls PhotoVideoControl from coming going on active vehicle gets rid of it before the camera manager on the vehicle goes away.

@HTRamsey Not sure if you have any thoughts here as well?

@HTRamsey
Copy link
Collaborator

That's something I kinda address in #13278. I don't personally think the prepare delete is necessary, but I made adjustments to prevent issues in unit tests in that PR

@julianoes
Copy link
Contributor

I think you two @DonLakeFlyer and @HTRamsey have a much better grasp on this than me. Sorry for use after free.

I remember the issue I was trying to fix was disconnecting and reconnecting cameras. I can re-test after the fixes have gone in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants