[FLINK-37637] Avoid dead lock for Configuration's addAll

[beliefer]

What is the purpose of the change

This PR aims to avoid dead lock for Configuration's addAll.
When multiple threads call the addAll method simultaneously and the passed Configuration objects are each other's parameters, a deadlock may occur:

// Thread 1
configA.addAll(configB);  // First lock configA.confData, then try locking configB.confData

// Thread 2
configB.addAll(configA);  // First lock configB.confData, then try locking configA.confData

Brief change log

Avoid dead lock for Configuration's addAll.

Verifying this change

This change is already covered by existing tests, such as (JobManagerProcessUtilsTest).
There are many test cases using the addAll.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
• The serializers: (no)
• The runtime per-record code paths (performance sensitive): (no)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
• The S3 file system connector: (no)

Documentation

• Does this pull request introduce a new feature? (no)

[flinkbot]

CI report:

• 06ef588 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[davidradl]

the locking sequence is the other way round in this instance

[beliefer]

It is always keep the same order of locking objects, so could avoid dead lock.

[davidradl]

I suggest lock1 -> thisConfDataLock
lock2 -> otherConfDataLock

[beliefer]

How about thisLock and otherLock ?

[davidradl]

I am not totally convinced there are no issues with this locking strategy.

If we have 2 configuration object A and B, they would always lock the other then ourselves. From object A's and B's perspective the lock order is not always the same.

[beliefer]

This change makes we always lock one first and then another. As you said, one thread locks the other object, then all the threads will lock the other object first.

[davidradl]

Yes I see that is how you have coded it, but objects should always be locked in the same order. So Object A then Object B should be locked in that order, irrespective of which thread owns them.

If each thread does its own object first, then thread 1 could lock Object A , then thread 2 lock Object B, then thread 1 would be locked out of Object B, and vice versa.

I was thinking if the objects had unique hashes then always do the lower hash first to ensure the order was the same.

[beliefer]

Yes. Let me take an example. There are two objects A and B. The hash value of Object A is smaller than Object B's.
Thread 1 holds the Object A and thread 2 holds the Object B. Thread 1 will lock Object A first and then Object B. Thread 2 wants lock Object B first but it can't since the hash value of Object A is lower than Object B, so this thread still needs lock Object A first and then Object B.

[beliefer]

@flinkbot run azure

[beliefer]

ping @StephanEwen cc @1996fanrui @davidradl @GOODBOY008

[beliefer]

@davidradl Could you take a look at this PR again ?