Pops::Loaders thread safety: synchronization in find_loader and []

[jstange]

Re: #9252

This patch makes reads and writes to Puppet::Pops::Loaders' instance variable @module_resolver thread-safe.

Without this, enabling threading on our stock Puppet 7 masters yields errors of the following form, on ~3-5% of agent runs:

ERROR [qtp735649759-289] [puppetserver] Puppet Server Error: Evaluation Error: Error while evaluating a Type-Name, Internal Error: Attempt to redefine loader named '<foo> private' (file: /path/to/our/modules/foo/manifests/init.pp, line: 12, column: 18) on node clientnode.example.com

(I theorize that this a race condition between multiple modules in our ecosystem resolving dependencies-in-common at the same time)

Applying this patch does seem to correct the behavior. We've been running this for a while on two of our active masters with no ill effects.

It seems likely that there are other thread-shared instance variables that should be wrapped in environment.lock.synchronize. I'm not particularly familiar with this code, so I stuck with the method that was trigger errors for us, as well as a nearby sibling, where it was clear that there was no danger of introducing deadlocks.

[puppetlabs-jenkins]

Can one of the admins verify this patch?

[CLAassistant]

All committers have signed the CLA.