michaeljmarshall opened a new pull request #13993: URL: https://github.com/apache/pulsar/pull/13993
### Motivation In preparing https://github.com/apache/pulsar/pull/13951 (still a draft at this time), I noticed that the `Backoff` class is not thread safe, but it is accessed as if it were because we pass it to multiple threads and call `.next()` to get the next backoff delay. ### Modifications * Move assertions for `initial` and `max` values to constructor and remove them from the `RetryUtil` class. * Remove the `@Data` annotation on the `Backoff` class. It allowed for unsafe access to internal state. * Add Javadocs to improve documentation of the class. * Remove unsafe initialization for the `next` variable. Here is my source showing that this initialization is unsafe: https://shipilev.net/blog/2014/safe-public-construction/. ### Verifying this change There is already test coverage for this class. The primary changes just add `synchronized` keywords to methods that get or update mutable state. ### Does this pull request potentially affect one of the following parts: It's possible that removing the `@Data` annotation on the class will break client applications since `Backoff` is a public class in the java client. I'll need guidance on how we should proceed here. ### Documentation - [x] `doc` This PR contains Javadocs for relevant documentation. There is no need to document this change elsewhere. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
