[BUG] Parts of Rest High-Level Client not thread-safe

[dennisoelkers]

Describe the bug

We were working on an issue in Graylog today when we found out that it is a deadlock between two threads which are stuck in the OpenSearch Rest High-Level Client. The underlying issue is that we have two threads trying to perform (different!) requests to OpenSearch at the same time, which are implicitly making use of the Settings class. This class is inherently unsafe in multi-threaded usage, due to its use of static variables/static Builder class/the circular reference between the outer class and the static nested class.

Our pragmatic fix was to make sure that both threads are started at different times, so initialization of the Settings class through thread 1 should be done when thread 2 starts, mitigating the problem for us. This is the related PR explaining and working around the issue.

It would be better though, if you would restructure the class to remove any of the factors causing a potential deadlock when it is used in two different threads simultaneous.

To Reproduce
Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Plugins
Please list all plugins currently enabled.

Screenshots
If applicable, add screenshots to help explain your problem.

Host/Environment (please complete the following information):

• OS: [e.g. iOS]
• Version [e.g. 22]

Additional context
Add any other context about the problem here.

[dblock]

@dennisoelkers Is there a way you can use a different transport instead of trying to fix this? We're trying to deprecate HLRS from https://github.com/opensearch-project/opensearch-java.

[dennisoelkers]

Hey @dblock,

not right now. We will switch over to the new client at some point, but it will take significant effort, so we need a short-term solution for the deadlock first.

[msfroh]

I have a scrappy fix that I can post a PR for.

It's essentially:

1. Settings.EMPTY = new Settings(Collections.emptyMap(), null); -- so loading the Settings class doesn't depend on loading Settings.Builder.
2. Settings.Builder.EMPTY_SETTINGS = Settings.EMPTY -- so we do have a dependency from Settings.Builder to Settings, but that's okay.