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.

Sign up for GitHub

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

Jump to bottom
Merged
gsakakihara merged 1 commit into from
Nov 17, 2022
Merged

Fix deadlock when handling simultaneous messages. #4327

gsakakihara merged 1 commit into from
Nov 17, 2022

Conversation

@gsakakihara
Copy link
Contributor

@gsakakihara gsakakihara commented Nov 15, 2022

#4315

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

@gsakakihara
Fix deadlock when handling simultaneous messages.
01ee1bd 
#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
Collaborator

google-oss-bot commented Nov 15, 2022 •
edited
Loading

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

github-actions bot commented Nov 15, 2022

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
Collaborator

google-oss-bot commented Nov 15, 2022

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

mkubiczek commented Nov 17, 2022

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
@gsakakihara
Fix deadlock when handling simultaneous messages. (#4327)
ca32190 
#4315
* Released sendWakefulServiceIntent()'s WakeLock on the main thread instead of within WithinAppServiceConnection to prevent a deadlock trying to acquire the WakeLockHolder.syncObject.
@gsakakihara gsakakihara mentioned this pull request Dec 7, 2022
Closed
@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.

Reviewers

@welishr welishr welishr approved these changes

@willxu-google willxu-google Awaiting requested review from willxu-google

@James201311 James201311 Awaiting requested review from James201311

Assignees

No one assigned

Labels

size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gsakakihara @google-oss-bot @mkubiczek @welishr