Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix : LogManager.ReconfigureExistingLoggers() could throw InvalidOperationException #2131

Merged
merged 2 commits into from
May 21, 2017

Conversation

jpdillingham
Copy link
Contributor

@jpdillingham jpdillingham commented May 21, 2017

…tently throws InvalidOperationException on Travis/Mono.

fixes #2130

…rmittently throws InvalidOperationException on Travis/Mono.
@codecov
Copy link

codecov bot commented May 21, 2017

Codecov Report

Merging #2131 into master will decrease coverage by <1%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #2131    +/-   ##
=======================================
- Coverage      81%     81%   -<1%     
=======================================
  Files         291     291            
  Lines       20103   20104     +1     
  Branches     2384    2384            
=======================================
- Hits        16342   16329    -13     
- Misses       3150    3163    +13     
- Partials      611     612     +1

@snakefoot
Copy link
Contributor

snakefoot commented May 21, 2017

Alternative one could change this method:

        public void ReconfigExistingLoggers()
        {
            List<Logger> loggers;
            lock (this.syncRoot)
            {
                if (this.config != null)
                {
                    this.config.InitializeAll();
                }

                //new list to avoid "Collection was modified; enumeration operation may not execute"
                loggers = new List<Logger>(loggerCache.Loggers);
            }

            foreach (var logger in loggers)
            {
                logger.SetConfiguration(this.GetConfigurationForLogger(logger.Name, this.config));
            }
        }

@304NotModified 304NotModified added bug Bug report / Bug fix enhancement Improvement on existing feature labels May 21, 2017
@304NotModified 304NotModified added this to the 4.4.10 milestone May 21, 2017
@304NotModified 304NotModified changed the title Fix for issue #2130: LogManager.ReconfigureExistingLoggers() intermit… Fix : LogManager.ReconfigureExistingLoggers() could throw InvalidOperationException May 21, 2017
@304NotModified
Copy link
Member

Thanks!

@snakefoot I think this is also OK. Thanks for the suggestion.

@304NotModified 304NotModified self-requested a review May 21, 2017 12:12
@304NotModified 304NotModified merged commit 252805e into NLog:master May 21, 2017
@jpdillingham jpdillingham deleted the issue-2130 branch May 21, 2017 15:19
@snakefoot
Copy link
Contributor

snakefoot commented May 21, 2017

Then one might consider changing:

List<WeakReference> loggerReferences = new List<WeakReference>(loggerCache.Values.ToList());

Into:

List<WeakReference> loggerReferences = loggerCache.Values.ToList();

And if being super picky, then one could perform reverse-iteration, and use RemoveAt for WeakReference no longer active.

@304NotModified
Copy link
Member

@snakefoot PR would be nice;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix enhancement Improvement on existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants