Log4net
  1. Log4net
  2. LOG4NET-212

Threading bug in the PatternConverter.cs

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.10
    • Fix Version/s: 1.2.11
    • Component/s: Core
    • Labels:
      None
    • Environment:
      Windows, .NET 2.0 / 3.5, C#

      Description

      Every once in a while I get the following exception:

      System.ArgumentOutOfRangeException: Index and length must refer to a location within the string.
      Parameter name: length
      at System.String.InternalSubStringWithChecks(Int32 startIndex, Int32 length, Boolean fAlwaysCopy)
      at System.Text.StringBuilder.ToString(Int32 startIndex, Int32 length)
      at log4net.Util.PatternConverter.Format(TextWriter writer, Object state) in xxx\Log4Net\src\Util\PatternConverter.cs:line 187
      at log4net.Layout.PatternLayout.Format(TextWriter writer, LoggingEvent loggingEvent) in xxx\Log4Net\src\Layout\PatternLayout.cs:line 1009
      at Nemmco.Common.Initialization.Internal.NemLoggingAppender.Execute(DateTime lastTrigger, DateTime currentTrigger) in xxxInitialization\Internal\InitializationLogging.cs:line 765
      -snip-

      From my own investigation it looks like the problem occurs because the shared string buffer (from the m_formatWriter.GetStringBuilder() call) may end up in a state where its size is adjusted differently on separate threads, causing one thread to over-estimate the available length.

      I wonder if the re-use of a StringWriter / StringBuilder in this scenario actually outweighs the threading implications? The simplest fix would be to replace use of m_formatWriter with use of a local StringWriter / StringBuilder.

        Activity

        Hide
        Stefan Bodewig added a comment - - edited

        The same is true for AppenderSkeleton.RenderLoggingEvent(LoggingEvent).

        I agree that proper locking would impact performance more than simply creating new StringWriters as needed. I'll set up some simplistic multi-threaded test scenario.

        Show
        Stefan Bodewig added a comment - - edited The same is true for AppenderSkeleton.RenderLoggingEvent(LoggingEvent). I agree that proper locking would impact performance more than simply creating new StringWriters as needed. I'll set up some simplistic multi-threaded test scenario.
        Hide
        Stefan Bodewig added a comment -

        I haven't found the time to write a testcase but on second thought the driver behind the shared StringWriter could be memory pressured environments (compact framework) and I wouldn't have a test environment to verify the impact there.

        Show
        Stefan Bodewig added a comment - I haven't found the time to write a testcase but on second thought the driver behind the shared StringWriter could be memory pressured environments (compact framework) and I wouldn't have a test environment to verify the impact there.
        Hide
        Stefan Bodewig added a comment -

        The immediate multi-threading problem is fixed with svn revsion 1169711

        I've opened LOG4NET-312 to review whether we actually need ReusableStringWriter at all in a future release.

        Show
        Stefan Bodewig added a comment - The immediate multi-threading problem is fixed with svn revsion 1169711 I've opened LOG4NET-312 to review whether we actually need ReusableStringWriter at all in a future release.

          People

          • Assignee:
            Unassigned
            Reporter:
            Jerry van Leeuwen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development