Log4net
  1. Log4net
  2. LOG4NET-232

Use ReaderWriterLockSlim instead of ReaderWriterLock.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.10
    • Fix Version/s: 1.2.12
    • Component/s: None
    • Labels:
      None
    • Environment:
      Any

      Description

      ReaderWriterLock should be replaced with ReaderWriterLockSlim according to Microsoft for performance and simplification reasons.

      MSDN: http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlock.aspx

      The .NET Framework has two reader-writer locks, ReaderWriterLockSlim and ReaderWriterLock. ReaderWriterLockSlim is recommended for all new development. ReaderWriterLockSlim is similar to ReaderWriterLock, but it has simplified rules for recursion and for upgrading and downgrading lock state. ReaderWriterLockSlim avoids many cases of potential deadlock. In addition, the performance of ReaderWriterLockSlim is significantly better than ReaderWriterLock.

        Issue Links

          Activity

          Hide
          Piers Williams added a comment -

          This is a real pain for us, because we are attempting to use Chess (MS Research) to test for multithreaded behavoral correctness, and due to the deprication of ReaderWriterLock, Chess doesn't support it (and throws NotImplementedExceptions when it encounters it).

          So we're having to come up with evil ways to strip out log4net from our code under test. Unfortunately even when our log4net code path isn't exercised at runtime, static class ctors in log4net give us grief. Much of this seems to come down to LoggerManager hooking ProcessExit / DomainUnload in it's static class ctor (which is pure evil by the way - why can't you just wait until the first repository is created?), and it's in there that a ReaderWriterLock is used when the repositories are shut down. I'm currently trying to unwire them using reflection which is just horrible.

          Looks like just a few changes to log4net.Util.ReaderWriterLock are all that's needed...

          Show
          Piers Williams added a comment - This is a real pain for us, because we are attempting to use Chess (MS Research) to test for multithreaded behavoral correctness, and due to the deprication of ReaderWriterLock, Chess doesn't support it (and throws NotImplementedExceptions when it encounters it). So we're having to come up with evil ways to strip out log4net from our code under test. Unfortunately even when our log4net code path isn't exercised at runtime, static class ctors in log4net give us grief. Much of this seems to come down to LoggerManager hooking ProcessExit / DomainUnload in it's static class ctor (which is pure evil by the way - why can't you just wait until the first repository is created?), and it's in there that a ReaderWriterLock is used when the repositories are shut down. I'm currently trying to unwire them using reflection which is just horrible. Looks like just a few changes to log4net.Util.ReaderWriterLock are all that's needed...
          Hide
          Piers Williams added a comment - - edited

          Since ReaderWriterLockSlim is a .net 3.5 addition (in System.Core), building on .net 3.5 is required to fix this issue

          See https://issues.apache.org/jira/browse/LOG4NET-176

          Show
          Piers Williams added a comment - - edited Since ReaderWriterLockSlim is a .net 3.5 addition (in System.Core), building on .net 3.5 is required to fix this issue See https://issues.apache.org/jira/browse/LOG4NET-176
          Hide
          Cosmin Onea added a comment -

          I've added the ReaderWriterLockSlim support for .NET 4 and above.
          When building for .NET 4 or above, System.Core is referenced.

          Here is the pull request: https://github.com/apache/log4net/pull/3

          Show
          Cosmin Onea added a comment - I've added the ReaderWriterLockSlim support for .NET 4 and above. When building for .NET 4 or above, System.Core is referenced. Here is the pull request: https://github.com/apache/log4net/pull/3
          Hide
          Dominik Psenner added a comment -

          Fixed as of revision: 1490222

          Show
          Dominik Psenner added a comment - Fixed as of revision: 1490222
          Hide
          Mike Barry added a comment - - edited

          Just wanted to inform you that this broke support for older versions of windows including XP, Vista < SP2, 2008 server core, Server 2003, and any itanium platform. (.NET 4.0>)

          http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx

          Supported Platforms: Windows 8, Windows Server 2012, Windows 7, Windows Vista SP2, Windows Server 2008 (Server Core Role not supported), Windows Server 2008 R2 (Server Core Role supported with SP1 or later; Itanium not supported)

          Show
          Mike Barry added a comment - - edited Just wanted to inform you that this broke support for older versions of windows including XP, Vista < SP2, 2008 server core, Server 2003, and any itanium platform. (.NET 4.0>) http://msdn.microsoft.com/en-us/library/system.threading.readerwriterlockslim.aspx Supported Platforms: Windows 8, Windows Server 2012, Windows 7, Windows Vista SP2, Windows Server 2008 (Server Core Role not supported), Windows Server 2008 R2 (Server Core Role supported with SP1 or later; Itanium not supported)
          Hide
          Dominik Psenner added a comment -

          ReaderWriterLockSlim should only be available on .NET 4.0 and newer. Open a bug if that's not the case.

          Show
          Dominik Psenner added a comment - ReaderWriterLockSlim should only be available on .NET 4.0 and newer. Open a bug if that's not the case.
          Hide
          Stefan Bodewig added a comment -

          .NET 4.0 is available for XP, so this doesn't help. Mike is probably right and the change has broken support for people using the .NET 4.0 assembly on XP.

          I'll try to write a testcase for this on my XP VM and find a workaround.

          Show
          Stefan Bodewig added a comment - .NET 4.0 is available for XP, so this doesn't help. Mike is probably right and the change has broken support for people using the .NET 4.0 assembly on XP. I'll try to write a testcase for this on my XP VM and find a workaround.
          Hide
          Mike Barry added a comment -

          Actually I looked closer. I was looking at the 4.5 documentation which does not support XP at all.

          4.0 does, but the slim lock only on SP3 and above, 2003 SP 2 above.

          Here is the official list:

          Windows 7, Windows Vista SP1 or later, Windows XP SP3, Windows Server 2008 (Server Core not supported), Windows Server 2008 R2 (Server Core supported with SP1 or later), Windows Server 2003 SP2

          Show
          Mike Barry added a comment - Actually I looked closer. I was looking at the 4.5 documentation which does not support XP at all. 4.0 does, but the slim lock only on SP3 and above, 2003 SP 2 above. Here is the official list: Windows 7, Windows Vista SP1 or later, Windows XP SP3, Windows Server 2008 (Server Core not supported), Windows Server 2008 R2 (Server Core supported with SP1 or later), Windows Server 2003 SP2
          Hide
          Stefan Bodewig added a comment -

          In that case my VM won't help as it runs XP SP3.

          Do you know what kind of exception is raised on older systems?

          Show
          Stefan Bodewig added a comment - In that case my VM won't help as it runs XP SP3. Do you know what kind of exception is raised on older systems?
          Hide
          Mike Barry added a comment -

          I'll get one running and let you know.

          Show
          Mike Barry added a comment - I'll get one running and let you know.
          Hide
          Mike Barry added a comment -

          4.0 is only supported on sp3. Since you didn't enable it for .net 3.5 which does support xp sp2 all should be well in the world. Sorry for the concern!

          Show
          Mike Barry added a comment - 4.0 is only supported on sp3. Since you didn't enable it for .net 3.5 which does support xp sp2 all should be well in the world. Sorry for the concern!
          Hide
          Stefan Bodewig added a comment -

          no need to be sorry. And thank you for checking!

          Show
          Stefan Bodewig added a comment - no need to be sorry. And thank you for checking!

            People

            • Assignee:
              Dominik Psenner
              Reporter:
              Aron Weiler
            • Votes:
              2 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development