Expose logger properties on Logger

[304NotModified]

fixes #3422 + refactor

[codecov-io]

Codecov Report

Merging #3424 into release/4.6.4 will decrease coverage by <1%.
The diff coverage is 82%.

@@              Coverage Diff               @@
##           release/4.6.4   #3424    +/-   ##
==============================================
- Coverage             80%     80%   -<1%     
==============================================
  Files                358     358            
  Lines              28373   28517   +144     
  Branches            3785    3814    +29     
==============================================
+ Hits               22750   22838    +88     
- Misses              4537    4583    +46     
- Partials            1086    1096    +10

[snakefoot]

You are opening a door to race-condition-hell by returning an unprotected dictionary.

I recommend that you return IReadOnlyDictionary that only works on the platforms where it is known.

[304NotModified]

And that race condition isn't there with WithProperty/SetProperty

[snakefoot]

I know WithProperty/SetProperty are nice and sweet (because I made them so). But the evil user that starts calling the IDictionary-inteface to insert/remove elements will break everything.

[304NotModified]

Sorry, I don't see what it should break. Could you please give an example ?

[snakefoot]

Very confused that you don't know what happens if one enumerates a collection while another modifies the collection. Made some pseudo-code in new comment.

[snakefoot]

var logger = LogManager.GetLogger("Hello").WithProperty("Test", 0);
var thread = new Thread(() => {
for (int i = 1; i < 1000000; ++i)
{
   logger.Properties.Remove("Test");
   logger.Properties.Add("Test", i);
}
});
thread.Start();
for (int i = 0; i < 1000000; ++i)
      logger.Info("Hello World");  // Collection was modified

[304NotModified]

We could change it to a concurrent dictionary? (Could, not saying we should)

[snakefoot]

We could change it to a concurrent dictionary? (Could, not saying we should)

That would be a bad trade, regarding logging-performance. Since the current reason for adding Properties-properties is to make it easier to see the current values. And the logger will be punished on every logevent with concurrentdictionary.

Ofcourse if the IDictionary-logic just created for GDC was extracted then we would have a thread-safe idictionary that could be used here.

[snakefoot]

This is not good. As you are modifying the active dictionary.

[304NotModified]

not really, CreateContextPropertiesDictionary made a new reference. But anyway, I see this PR is already superseded 👼

[304NotModified]

superseded by #3430