launchdarkly / android-client-sdk

LaunchDarkly Client-side SDK for Android
Other
45 stars 23 forks source link

Do not syncrhonize Throttler::run #163

Closed res0nance closed 2 years ago

res0nance commented 2 years ago

Requirements

Related issues

See: #162

Describe the solution you've provided

Throttler::run and Throttler::attemptRun are the two functions that can syncrhonize both itself and ConnectivityManager in that order, ConnectivityManager itself usually syncrhonizes on itself then on Throttler by calling its functions. Because Throttler::run is run on a separate thread this becomes a problem as the order of locking is different. Removing synchronize from Throttler::run will restore the order as the runnable will lock ConnectivityManager. Throttler::attemptRun is only called from ConnectivityManager and after it locks ConnectivityManager so Throttler::attemptRun should not require any changes.

Describe alternatives you've considered

No alternatives considered

louis-launchdarkly commented 2 years ago

Hello @res0nance, thank you for the contribution. The main Android engineer is OOO, so it will take us some time before we can respond to this request. We will give you another update once we discussed the change.

res0nance commented 2 years ago

Hey @louis-launchdarkly any update here?

res0nance commented 2 years ago

@louis-launchdarkly this has been open for about a month and a half, can this be reviewed please?