Skip to content

Fix deadlock when handling simultaneous messages. #4327

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

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

gsakakihara
Copy link
Contributor

#4315

  • Released sendWakefulServiceIntent()'s WakeLock on the main thread instead of within WithinAppServiceConnection to prevent a deadlock trying to acquire the WakeLockHolder.syncObject.

#4315
* Released sendWakefulServiceIntent()'s WakeLock on the main thread instead of within WithinAppServiceConnection to prevent a deadlock trying to acquire the WakeLockHolder.syncObject.
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 15, 2022

Coverage Report 1

Affected Products

  • firebase-messaging

    Overall coverage changed from ? (c1adf5b) to 85.14% (c831bfd) by ?.

    41 individual files with coverage change

    FilenameBase (c1adf5b)Merge (c831bfd)Diff
    AutoProtoEncoderDoNotUseEncoder.java?98.17%?
    ByteStreams.java?59.72%?
    CommonNotificationBuilder.java?90.78%?
    Constants.java?92.86%?
    DisplayNotification.java?91.38%?
    EnhancedIntentService.java?76.60%?
    ExecutorFactory.java?0.00%?
    FcmBroadcastProcessor.java?93.33%?
    FcmExecutors.java?80.00%?
    FcmLifecycleCallbacks.java?89.29%?
    FirebaseInstanceIdReceiver.java?72.73%?
    FirebaseMessaging.java?76.21%?
    FirebaseMessagingRegistrar.java?100.00%?
    FirebaseMessagingService.java?94.03%?
    GmsRpc.java?83.15%?
    ImageDownload.java?93.33%?
    MessagingAnalytics.java?81.78%?
    MessagingClientEvent.java?93.58%?
    MessagingClientEventExtension.java?76.47%?
    Metadata.java?57.14%?
    NotificationParams.java?98.71%?
    PoolableExecutors.java?29.17%?
    ProtoEncoderDoNotUse.java?50.00%?
    ProxyNotificationInitializer.java?93.75%?
    ProxyNotificationPreferences.java?88.89%?
    RemoteMessage.java?95.48%?
    RemoteMessageCreator.java?88.89%?
    RequestDeduplicator.java?80.00%?
    SendException.java?100.00%?
    ServiceStarter.java?87.50%?
    SharedPreferencesQueue.java?98.65%?
    Store.java?74.19%?
    SyncTask.java?70.83%?
    ThreadPriority.java?100.00%?
    TopicOperation.java?90.00%?
    TopicsStore.java?100.00%?
    TopicsSubscriber.java?89.47%?
    TopicsSyncTask.java?53.33%?
    WakeLockHolder.java?95.92%?
    WithinAppServiceBinder.java?90.91%?
    WithinAppServiceConnection.java?84.15%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/DrNopU2rdi.html

@github-actions
Copy link
Contributor

Unit Test Results

  27 files   -    368    27 suites   - 368   4m 3s ⏱️ - 16m 33s
474 tests  - 4 256  474 ✔️  - 4 234  0 💤  - 22  0 ±0 
474 runs   - 4 272  474 ✔️  - 4 250  0 💤  - 22  0 ±0 

Results for commit 01ee1bd. ± Comparison against base commit c1adf5b.

@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • base

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.65 kB? (?)
  • firebase-annotations

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.89 kB? (?)
  • firebase-common

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?50.1 kB? (?)
    apk (aggressive)?85.1 kB? (?)
    apk (release)?683 kB? (?)
  • firebase-components

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?42.8 kB? (?)
    apk (aggressive)?8.68 kB? (?)
    apk (release)?31.9 kB? (?)
  • firebase-datatransport

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?4.94 kB? (?)
    apk (aggressive)?136 kB? (?)
    apk (release)?771 kB? (?)
  • firebase-encoders

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    apk (aggressive)?8.68 kB? (?)
    apk (release)?15.3 kB? (?)
  • firebase-encoders-json

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?10.7 kB? (?)
    apk (aggressive)?8.68 kB? (?)
    apk (release)?20.1 kB? (?)
  • firebase-encoders-proto

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    apk (aggressive)?8.68 kB? (?)
    apk (release)?21.6 kB? (?)
  • firebase-installations

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?54.9 kB? (?)
    apk (aggressive)?86.5 kB? (?)
    apk (release)?706 kB? (?)
  • firebase-installations-interop

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?8.05 kB? (?)
    apk (aggressive)?65.0 kB? (?)
    apk (release)?651 kB? (?)
  • firebase-messaging

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?142 kB? (?)
    apk (aggressive)?440 kB? (?)
    apk (release)?1.14 MB? (?)
  • transport-api

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?6.74 kB? (?)
    apk (aggressive)?8.68 kB? (?)
    apk (release)?14.9 kB? (?)
  • transport-backend-cct

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?53.6 kB? (?)
    apk (aggressive)?58.2 kB? (?)
    apk (release)?105 kB? (?)
  • transport-runtime

    TypeBase (c1adf5b)Merge (c831bfd)Diff
    aar?180 kB? (?)
    apk (aggressive)?44.0 kB? (?)
    apk (release)?83.5 kB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/eiQMmzfQxe.html

@mkubiczek
Copy link

Can I ask to get some attention to this PR please? ;) We are affected a lot by this issue.

@gsakakihara gsakakihara merged commit 94ae2ac into master Nov 17, 2022
@gsakakihara gsakakihara deleted the gsakakihara/deadlock branch November 17, 2022 14:21
davidmotson pushed a commit that referenced this pull request Nov 28, 2022
#4315
* Released sendWakefulServiceIntent()'s WakeLock on the main thread instead of within WithinAppServiceConnection to prevent a deadlock trying to acquire the WakeLockHolder.syncObject.
@firebase firebase locked and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants