Apache Commons Configuration and thread safety

[Nadahar]

@UniversalMediaServer/developers
While working on #616 adding a getter to PmsConfiguration that needs to be synchronized I began wondering about thread safety and the PmsConfiguration class in general (and whether or not I needed to add synchronization to this method, or if it was handled "behind the scenes somewhere").

The class seems unnecessary complex to me, and I don't understand why it's based on RendererConfiguration at all. But, from what I can find it's not synchronized manually in any way, and the underlying Apache Commons Configuration isn't synchronized either. If I didn't miss anything, that means that both PmsConfiguration and RendererConfiguration and any other similar classes are unsafe and will generate random bugs.

Apache Commons Configuration version 2.0 has synchronization handling via the ReadWriteSynchronizer, but since this is only in beta I guess upgrading is not very appealing?

One reason the problems related to this are relatively few might be because most properties are mostly read and rarely written but that doesn't mean it's safe.

What am I missing?

[skeptical]

I don't understand why it's based on RendererConfiguration at all

@Nadahar I'm the guilty party :-).

Short answer is retrofitting composite model onto older unrelated PmsConfiguration and RendererConfiguration classes so DeviceConfiguration could inherit both sets of getters, see this sequence of 9 commits (cb39545, 460d233, b824f38, 7c26b1c, fe4dbfb thru 46614e2) and my explanation here. A bit convoluted, but the point (such as it is) was to keep these key classes externally the same so as not to disrupt the ongoing work of other devs too much.

[Sami32]

BTW it's not any more in "beta".

[Nadahar]

Thread safety is overrated.

[SubJunk]

We still aspire to have thread safety even if we don't always have the time to spend on it. Good to have a record of the bug in case anyone finds time for it.