Skip to content
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

NPE when loading the duration results back to Flutter #1372

Closed
snipd-min opened this issue Dec 2, 2024 · 11 comments · Fixed by #1373
Closed

NPE when loading the duration results back to Flutter #1372

snipd-min opened this issue Dec 2, 2024 · 11 comments · Fixed by #1373
Assignees
Labels
1 backlog bug Something isn't working

Comments

@snipd-min
Copy link

Which API doesn't behave as documented, and how does it misbehave?
In the - (void)load:(NSDictionary *)source initialPosition:(CMTime)initialPosition initialIndex:(NSNumber *)initialIndex result:(FlutterResult)result method, the _loadResult(@{@"duration": @([self getDurationMicroseconds])}); at line 693 could throw a NPE when the _loadResult is already reported via observeValueForKeyPath at line 814.

if (_indexedAudioSources.count == 0 || !_player.currentItem ||
            _player.currentItem.status == AVPlayerItemStatusReadyToPlay) {
        _processingState = ready;
        _loadResult(@{@"duration": @([self getDurationMicroseconds])});      // <--- the NPE happened here
        _loadResult = nil;
    } else {
        // We send result after the playerItem is ready in observeValueForKeyPath.
    }

I'm not sure if observeValueForKeyPath should ever report the duration before the load method, but here in theory the _loadResult could be set to nil and should probably check for it before calling.

Here I attached the crash log trace from xcode via the Organizer > Crashes:

image

Minimal reproduction project
Unfortunately I couldn't find a way to reproduce this after a lot of tries. I still feel like I should report it as it's quite clear what actually happened here (or should have happened). This only happens to a very small fraction of users that we have.

To Reproduce (i.e. user steps, not code)
Based on the breadcrumbs, it happens just by playing the audio.

Error messages

Crashed: com.apple.main-thread
EXC_BAD_ACCESS KERN_INVALID_ADDRESS 0x0000000000000010

Crashed: com.apple.main-thread
0  just_audio                     0x74cc -[AudioPlayer load:initialPosition:initialIndex:result:] + 1900
1  just_audio                     0x47ec -[AudioPlayer handleMethodCall:result:] + 524
2  just_audio                     0x45a8 __70-[AudioPlayer initWithRegistrar:playerId:loadConfiguration:userAgent:]_block_invoke + 72
3  Flutter                        0x5c0a8c InternalFlutterGpu_Texture_AsImage + 6316
4  Flutter                        0x43d0c (Missing UUID 4c4c44ef55553144a1c675e6577e3daa)
5  libdispatch.dylib              0x213c _dispatch_call_block_and_release + 32
6  libdispatch.dylib              0x3dd4 _dispatch_client_callout + 20
7  libdispatch.dylib              0x125a4 _dispatch_main_queue_drain + 988
8  libdispatch.dylib              0x121b8 _dispatch_main_queue_callback_4CF + 44
9  CoreFoundation                 0x56710 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 16
10 CoreFoundation                 0x53914 __CFRunLoopRun + 1996
11 CoreFoundation                 0x52cd8 CFRunLoopRunSpecific + 608
12 GraphicsServices               0x11a8 GSEventRunModal + 164
13 UIKitCore                      0x40aae8 -[UIApplication _run] + 888
14 UIKitCore                      0x4bed98 UIApplicationMain + 340
15 Runner                         0x97a8 main + 8 (AppDelegate.swift:8)
16 ???                            0x1c1f47154 (Missing)

Expected behavior
In case the duration is already reported via observeValueForKeyPath, the _loadResult doesn't need to be called and should just move on.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):

  • Device: iOS
  • OS: iOS 15-18

Flutter SDK version
[✓] Flutter (Channel stable, 3.24.3, on macOS 15.1.1 24B91 darwin-arm64, locale
en-US)
• Flutter version 3.24.3 on channel stable at
/Users/minzhao/fvm/versions/3.24.3
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision 2663184aa7 (3 months ago), 2024-09-11 16:27:48 -0500
• Engine revision 36335019a8
• Dart version 3.5.3
• DevTools version 2.37.3

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
• Android SDK at /Users/minzhao/Library/Android/sdk
• Platform android-34, build-tools 33.0.1
• ANDROID_HOME = /Users/minzhao/Library/Android/sdk
• ANDROID_SDK_ROOT = /Users/minzhao/Library/Android/sdk
• Java binary at: /Applications/Android
Studio.app/Contents/jbr/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build
17.0.11+0-17.0.11b1207.24-11852314)
• All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 16.1)
• Xcode at /Applications/Xcode.app/Contents/Developer
• Build 16B40
• CocoaPods version 1.15.2

[✓] Chrome - develop for the web
• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2024.1)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build
17.0.11+0-17.0.11b1207.24-11852314)

[✓] VS Code (version 1.95.3)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 3.98.0

[✓] Network resources
• All expected network resources are available.

Additional context
N/A

@snipd-min snipd-min added 1 backlog bug Something isn't working labels Dec 2, 2024
@ryanheise
Copy link
Owner

Thanks. Based on the information provided I should be able to investigate it.

(...after I get my new dev box up and running, which should hopefully be soon.)

@ryanheise
Copy link
Owner

Hi @snipd-min , I've created a PR #1373 . Would you be able to confirm that this doesn't break anything on your end?

@snipd-min
Copy link
Author

Hi @ryanheise, thanks for the quick feedback!

I've made a similar fix that we will soon roll out soon so we should see if the crash would stop happening. I didn't notice anything wrong from testing with the audio player. Will still probably test it in a few days to make sure nothing is wrong. Regarding the crash, although it's highly likely that it fixes that, it might take a while to see the affect in production as we would need to roll out the app and observe on Firebase. I can report it back asap though if it's still needed.

One thing I'm not certain is if the line after the loadResult, i.e. _processingState = ready; should still be executed in _load or should be added to observeValueForKeyPath.

I have kept that line as I was a bit afraid if this could be somehow missed in observeValueForKeyPath as it wasn't going to be set at the line where _loadResult is called, instead it would be when the observed key path is playbackBufferEmpty / playbackBufferFull.

@ryanheise
Copy link
Owner

One thing I'm not certain is if the line after the loadResult, i.e. _processingState = ready; should still be executed in _load or should be added to observeValueForKeyPath.

I have kept that line as I was a bit afraid if this could be somehow missed in observeValueForKeyPath as it wasn't going to be set at the line where _loadResult is called, instead it would be when the observed key path is playbackBufferEmpty / playbackBufferFull.

I'd be inclined to leave it in as a sane initialiser. I'm not able to test things right now, but my thinking is that if there truly is buffering to happen, observeValueForKeyPath will be called and the correct state will ultimately be set. But on the other hand, if no subsequent observeValueForKeyPath calls will ever happen because the item was immediately ready, then this will at least leave things prevailing in the correct state.

I've made a similar fix that we will soon roll out soon so we should see if the crash would stop happening. I didn't notice anything wrong from testing with the audio player. Will still probably test it in a few days to make sure nothing is wrong.

Are there any material differences between your fix and this PR?

@snipd-min
Copy link
Author

Make sense, that was the only difference I have.

@jvelezos
Copy link

jvelezos commented Dec 12, 2024

I had a crash today pointing to the same exact line. I'm guessing it's coming from the same mentioned issue.

Here's the crash log

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_INVALID_ADDRESS at 0x0000000000000010
Exception Codes: 0x0000000000000001, 0x0000000000000010
VM Region Info: 0x10 is not in any region.  Bytes before following region: 4300505072
      REGION TYPE                 START - END      [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      UNUSED SPACE AT START
--->  
      __TEXT                   100548000-10057c000 [  208K] r-x/r-x SM=COW  /var/containers/Bundle/Application//Runner.app/Runner
Termination Reason: SIGNAL 11 Segmentation fault: 11
Terminating Process: exc handler [4215]

Triggered by Thread:  0


Thread 0 name:
Thread 0 Crashed:
0   just_audio                    	0x0000000102a9b5fc -[AudioPlayer load:initialPosition:initialIndex:result:] + 1900 (AudioPlayer.m:693)
1   just_audio                    	0x0000000102a987c8 -[AudioPlayer handleMethodCall:result:] + 524 (AudioPlayer.m:123)
2   just_audio                    	0x0000000102a98584 __60-[AudioPlayer initWithRegistrar:playerId:loadConfiguration:]_block_invoke + 72 (AudioPlayer.m:113)
3   Flutter                       	0x0000000105048b50 __45-[FlutterMethodChannel setMethodCallHandler:]_block_invoke + 164 (FlutterChannels.mm:313)
4   Flutter                       	0x0000000104ad32e8 invocation function for block in flutter::PlatformMessageHandlerIos::HandlePlatformMessage(std::_fl::unique_ptr<flutter::PlatformMessage, std::_fl::default_delete<flutter::PlatformMessage>>) + 116 (platform_message_handler_ios.mm:70)
5   libdispatch.dylib             	0x0000000193289248 _dispatch_call_block_and_release + 32 (init.c:1549)
6   libdispatch.dylib             	0x000000019328afa8 _dispatch_client_callout + 20 (object.m:576)
7   libdispatch.dylib             	0x0000000193299a34 _dispatch_main_queue_drain + 984 (queue.c:8093)
8   libdispatch.dylib             	0x000000019329964c _dispatch_main_queue_callback_4CF + 44 (queue.c:8253)
9   CoreFoundation                	0x000000018b54fbbc 0x18b4d6000 + 498620
10  CoreFoundation                	0x000000018b54c1b0 0x18b4d6000 + 483760
11  CoreFoundation                	0x000000018b59e274 0x18b4d6000 + 819828
12  GraphicsServices              	0x00000001d86d54c0 0x1d86d4000 + 5312
13  UIKitCore                     	0x000000018e0eb480 0x18dcfc000 + 4125824
14  UIKitCore                     	0x000000018dd11410 0x18dcfc000 + 87056
15  Runner                        	0x000000010057e388 main + 64 (AppDelegate.swift:15)
16  dyld                          	0x00000001b16c2de8 0x1b1693000 + 196072

@ryanheise
Copy link
Owner

I guess I can merge this fix, but @jvelezos would you be able to test it on your end to confirm that at the very least it does not break your app?

@jvelezos
Copy link

@ryanheise Your PR is not breaking anything on my side 👍

@ryanheise
Copy link
Owner

Thanks for confirming, this has now been merged and released.

@ryanheise
Copy link
Owner

Sorry, I mean merged, but not yet released.

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs, or use StackOverflow if you need help with just_audio.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 backlog bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants