Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

fix npe in closeCaptureSession #7197

Closed
wants to merge 1 commit into from
Closed

Conversation

emakar
Copy link

@emakar emakar commented Feb 20, 2023

This is a resubmission of #6620

We have such npe:

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.hardware.camera2.CameraCaptureSession.close()' on a null object reference
	at io.flutter.plugins.camera.Camera.closeCaptureSession(Camera.java:1244)
	at io.flutter.plugins.camera.Camera$1.onClosed(Camera.java:372)
	at android.hardware.camera2.impl.CameraDeviceImpl$5.run(CameraDeviceImpl.java:234)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:210)
	at android.os.Looper.loop(Looper.java:299)
	at android.os.HandlerThread.run(HandlerThread.java:67)

This code is racy, so I've added a simple protection to it and similar places.

Fixes flutter/flutter#121079.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@emakar emakar requested a review from camsim99 as a code owner February 20, 2023 08:09
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@stuartmorgan-g
Copy link
Contributor

Thanks for the contribution!

This code is racy

Please describe the race in detail, either here or in the issue. How specifically is it happening, and what are the threads involved?

so I've added a simple protection to it and similar places.

I'm not seeing how this is a fix; if this code is being run simultaneously from two different threads then you've just replaced an NPE with API violations by double-calling methods that violate the camera state flow. Am I missing something about the change?

  • I added new tests to check the change I am making, or this PR is test-exempt.

Where is this test?

@stuartmorgan-g
Copy link
Contributor

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

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

Successfully merging this pull request may close these issues.

[camera] NPE in CameraCaptureSession.close()
2 participants