Skip to content

Do not crash when installing shutdown hook#3456

Merged
romtsn merged 2 commits into
mainfrom
rz/fix/shutdownhook-too-late
Jun 4, 2024
Merged

Do not crash when installing shutdown hook#3456
romtsn merged 2 commits into
mainfrom
rz/fix/shutdownhook-too-late

Conversation

@romtsn

@romtsn romtsn commented Jun 3, 2024

Copy link
Copy Markdown
Member

📜 Description

  • There was a report from the SDK console (See below), so I've just wrapped the addShutdownHook method analogous to what we do when removing the hook

💡 Motivation and Context

Google Play SDK console yield this crash:

Exception java.lang.IllegalStateException: VM already shutting down
  at java.lang.Runtime.addShutdownHook (Runtime.java:280)
  at io.sentry.ShutdownHookIntegration.register (ShutdownHookIntegration.java:34)
  at io.sentry.Sentry.init (Sentry.java:237)
  at io.sentry.Sentry.init (Sentry.java:147)
  at io.sentry.android.core.SentryAndroid.init (SentryAndroid.java:89)
  at io.sentry.android.core.SentryAndroid.init (SentryAndroid.java:70)
  at <private>
  at java.lang.reflect.Method.invoke
  at <private>
  at android.os.Handler.handleCallback (Handler.java:1000)
  at android.os.Handler.dispatchMessage (Handler.java:104)
  at <private>
  at android.os.Looper.loopOnce (Looper.java:242)
  at android.os.Looper.loop (Looper.java:362)
  at <private>
  at java.lang.Thread.run (Thread.java:1012)

💚 How did you test it?

unit tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@github-actions

github-actions Bot commented Jun 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 482.89 ms 597.22 ms 114.33 ms
Size 1.70 MiB 2.28 MiB 592.39 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d6d2b2e 392.55 ms 467.50 ms 74.95 ms
c7e2fbc 393.98 ms 478.24 ms 84.27 ms
4e260b3 388.40 ms 468.98 ms 80.58 ms
28c9a83 366.20 ms 433.88 ms 67.67 ms
baaf637 418.39 ms 496.86 ms 78.47 ms
283d83e 416.81 ms 497.22 ms 80.41 ms
283d83e 348.44 ms 392.06 ms 43.62 ms
0bf143e 368.35 ms 437.47 ms 69.12 ms
86f0096 368.63 ms 446.92 ms 78.29 ms
619c9b9 428.23 ms 514.66 ms 86.43 ms

App size

Revision Plain With Sentry Diff
d6d2b2e 1.72 MiB 2.27 MiB 555.05 KiB
c7e2fbc 1.72 MiB 2.29 MiB 576.40 KiB
4e260b3 1.72 MiB 2.27 MiB 554.95 KiB
28c9a83 1.70 MiB 2.28 MiB 592.00 KiB
baaf637 1.72 MiB 2.27 MiB 558.42 KiB
283d83e 1.72 MiB 2.29 MiB 577.69 KiB
283d83e 1.72 MiB 2.29 MiB 577.69 KiB
0bf143e 1.72 MiB 2.29 MiB 576.50 KiB
86f0096 1.72 MiB 2.29 MiB 576.50 KiB
619c9b9 1.70 MiB 2.28 MiB 592.12 KiB

Previous results on branch: rz/fix/shutdownhook-too-late

Startup times

Revision Plain With Sentry Diff
fbd7a40 348.85 ms 424.92 ms 76.07 ms

App size

Revision Plain With Sentry Diff
fbd7a40 1.70 MiB 2.28 MiB 592.39 KiB

adinauer
adinauer approved these changes Jun 4, 2024
View reviewed changes

@adinauer adinauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@romtsn
romtsn force-pushed the rz/fix/shutdownhook-too-late branch from 355cf10 to 3464578 Compare June 4, 2024 14:16
@romtsn
romtsn enabled auto-merge (squash) June 4, 2024 14:16
@romtsn
romtsn merged commit 84892a8 into main Jun 4, 2024
@romtsn
romtsn deleted the rz/fix/shutdownhook-too-late branch June 4, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adinauer adinauer adinauer approved these changes
@stefanosiano stefanosiano Awaiting requested review from stefanosiano
@markushi markushi Awaiting requested review from markushi markushi is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@romtsn @adinauer