fix: some crashes have come up because of logger injection. the reason seems to be the the binder is being updated every inject

[thomaszurkan-optimizely]

Summary

There have been a small number of crashes reported and it appears to be the injection of loggers from multiple threads at the same time. This is because the binder was updating the instance variable even when the binder was always suppling new instances (i.e. when not a singleton).

The fix is two fold:

1. If it is not a singleton just return a new instance via the factory and return.
2. Convert Binders to structs so there is a copy on read and a copy on write. This way, any changes to the binders are changes to a copy and then the updating of the binder goes through an atomic property so the setter is atomic.

[coveralls]

Coverage decreased (-0.09%) to 98.662% when pulling ab315a7 on binderFix into f1995d5 on master.

[jaeopt]

It looks good for fixing the logger issue.
We still have unsafe dictionaries as other possible sync failure sources. Can we fix them all?

[jaeopt]

This is redundant with the separate CustomLogger file. I see you changed it to private. Is it required?

[thomaszurkan-optimizely]

No, I just wanted to make sure it wasn't a problem if it was private.

[jaeopt]

Then let's keep it internal and remove this extra copy (or the other one).

[jaeopt]

Don't we need synchronization to update binders dictionary?
Let's fix this unsafe dictionaries/arrays in other places as well.

[thomaszurkan-optimizely]

the binder set is an atomic property. the binder is a copy and so this is an atomic operation. the get and set are separate. but, the again, since this is only an issue with a singleton that usually provides a instance with creation. it should not be an issue. i don't want to slow everything down by constantly wrapping everything in a dispatch.

[thomaszurkan-optimizely]

It looks good for fixing the logger issue.
We still have unsafe dictionaries as other possible sync failure sources. Can we fix them all?

Can you tell me where we are accessing unsafe dictionaries? I would handle that with a synched dictionary not by augment ing the atomic property by the way.

[jaeopt]

The test is broken and cannot catch the thread issue here.

[jaeopt]

This sleep delay breaks the test. 1000 async tasks not concurrent any more because this delay.
If we remove the sleep delay and using DispatchGroup for sync, this test fails because concurrency issue.

[thomaszurkan-optimizely]

This sleep delay breaks the test. 1000 async tasks not concurrent any more because this delay.
If we remove the sleep delay and using DispatchGroup for sync, this test fails because concurrency issue.

Please add a test if you think it would break. The test won't run all the queued in the global queue without the sleep. So.....

[jaeopt]

Approved for logger bug fix.
The other thread issue I raised can be separated and will be reviewed later instead of blocking this fix.