Replace synchronized withReentrantLock

[adinauer]

📜 Description

• Most usages of synchronized keyword in method signatures and synchronized (...) { blocks have been replaced by using ReentrantLock
• Adds AutoClosableReentrantLock which offers an acquire method that locks and returns an AutoClosable that unlocks on close. This can be used in try blocks
• SynchronizedCollection and SynchronizedQueue now expect a AutoClosableReentrantLock instead of Object when passing a lock object
• Instance locks (previously synchronized methods and synchronized (this)) now use lock
• Static locks (previously static synchronized, synchronized (...class) and synchronized (...getInstance)) now use staticLock
• If there was a lock object before, the type has been changed from Object to AutoClosableReentrantLock
• Changing Android only classes wasn't totally necessary but IMO it's easier to maintain if there's only one way of synchronizing that's being used

💡 Motivation and Context

Fixes #3312

💚 How did you test it?

📝 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]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	436.66 ms	493.14 ms	56.48 ms
Size	1.70 MiB	2.35 MiB	668.82 KiB

Previous results on branch: fix/replace-synchronized-with-lock

Startup times

Revision	Plain	With Sentry	Diff
4a331fa	408.77 ms	477.06 ms	68.29 ms

App size

Revision	Plain	With Sentry	Diff
4a331fa	1.70 MiB	2.35 MiB	668.79 KiB

[markushi]

Nice one! 🥷

[markushi]

To be honest I don't know why we had all the sychronized blocks in here, it seems to stem from here. I think we can remove it in future.

Suggested change

	public ActivityBreadcrumbsIntegration(final @NotNull Application application) {
	// TODO check if locking is even required at all for lifecycle methods
	public ActivityBreadcrumbsIntegration(final @NotNull Application application) {

[markushi]

1. I don't think we need the locking here
2. Tell me how you automated that refactoring 😅

Suggested change

	try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
	// no-op
	}
	// no-op

[adinauer]

I had to take a look at each thing I refactored manually 😢 . My thinking was to make this more clear in case we replace the noop comment with some actual code in the future.

[adinauer]

Replaced the locking code with a comment that shall serve as a reminder

[markushi]

could this be private as well?

[adinauer]

No because it's used in SynchronizedQueue. The same is basically true for any non final class since you have to use the right lock instead of just relying on the synchronized keyword on methods.

[github-actions]

	Fails
🚫	Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Replace `synchronized` with`ReentrantLock` ([#3715](https://github.com/getsentry/sentry-java/pull/3715))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 2dfe2ae