Hmm, I don’t think adding an extra layer of indirection here is the right approach. Usually I’d agree that thread safety should be left up to the user of a class but in this case the type is pretty fundamental, used internally in a lot of juce classes/function calls and of a type that is extremely often used on background threads.
Reading files, parsing them, creating new ones is a perfect example of the type of thing that should be offloaded to background threads to keep the main thread free and your app responsive.
PropertiesFile::saveAsBinary etc. all use
juce::TemporaryFile internally. Are you saying that these methods should only be called from a single (probably the message) thread?
Performance wise, these
juce::TemporaryFile constructors call the
File::getNonexistentChildFile method which includes several disk accesses and system calls plus a ton of memory allocations and string manipulations. Creating a local
Random object is unlikely to have a significant performance impact on creating these objects.
If there’s some proof that keeping the few cycles makes a noticeable difference to a program that is creating and writing lots of files there’s other ways to make this thread-safe without passing on responsibility to the caller or creating a local Random object. For example having a static function with a static Random object and a static spin lock would suffice and almost always be just the cost of two atomic stores as the contention is likely to be low.
I think the decorator pattern is a really poor choice for solving this problem, not only because every consumer of the library would have to implement it, they’d also have to copy some internal functions of
juce::TemporaryFile, what happens then if the internal behaviour of
juce::TemporaryFile changes? Every user of the library has to notice this and copy the internals again just to make sure their own special thread-safe version works the same way as the library implementation?
My fix was literally removing a few characters (
::getSystemRandom) in two places in the file and has an absolutely tiny performance hit in a non-performance critical piece of code. Let’s think about this before over-engineering an alternative solution.