Skip to content

Commit fb9235d

Browse files
committed
fixed CA2002: Do not lock on objects with weak identity - https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2002
1 parent 1296388 commit fb9235d

11 files changed

+231
-195
lines changed

src/log4net.Tests/Core/FixingTest.cs

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
namespace log4net.Tests.Core;
2929

3030
[TestFixture]
31+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2201:Do not raise reserved exception types")]
3132
public class FixingTest
3233
{
3334
const string TestRepository = "Test Repository";

src/log4net/Appender/AppenderSkeleton.cs

+6-4
Original file line numberDiff line numberDiff line change
@@ -45,16 +45,18 @@ namespace log4net.Appender;
4545
/// <author>Gert Driesen</author>
4646
public abstract class AppenderSkeleton : IAppender, IBulkAppender, IOptionHandler, IFlushable
4747
{
48+
/// <summary>
49+
/// SyncRoot
50+
/// </summary>
51+
private protected object SyncRoot { get; } = new();
52+
4853
/// <summary>
4954
/// Default constructor
5055
/// </summary>
5156
/// <remarks>
5257
/// <para>Empty default constructor</para>
5358
/// </remarks>
54-
protected AppenderSkeleton()
55-
{
56-
_errorHandler = new OnlyOnceErrorHandler(this.GetType().Name);
57-
}
59+
protected AppenderSkeleton() => _errorHandler = new OnlyOnceErrorHandler(GetType().Name);
5860

5961
/// <summary>
6062
/// Finalizes this appender by calling the implementation's

src/log4net/Appender/BufferingForwardingAppender.cs

+5-5
Original file line numberDiff line numberDiff line change
@@ -89,23 +89,23 @@ protected override void SendBuffer(LoggingEvent[] events)
8989
/// Adds an <see cref="IAppender" /> to the list of appenders of this
9090
/// instance.
9191
/// </summary>
92-
/// <param name="newAppender">The <see cref="IAppender" /> to add to this appender.</param>
92+
/// <param name="appender">The <see cref="IAppender" /> to add to this appender.</param>
9393
/// <remarks>
9494
/// <para>
9595
/// If the specified <see cref="IAppender" /> is already in the list of
9696
/// appenders, then it won't be added again.
9797
/// </para>
9898
/// </remarks>
99-
public virtual void AddAppender(IAppender newAppender)
99+
public virtual void AddAppender(IAppender appender)
100100
{
101-
if (newAppender is null)
101+
if (appender is null)
102102
{
103-
throw new ArgumentNullException(nameof(newAppender));
103+
throw new ArgumentNullException(nameof(appender));
104104
}
105105
lock (LockObj)
106106
{
107107
_appenderAttachedImpl ??= new AppenderAttachedImpl();
108-
_appenderAttachedImpl.AddAppender(newAppender);
108+
_appenderAttachedImpl.AddAppender(appender);
109109
}
110110
}
111111

src/log4net/Appender/ColoredConsoleAppender.cs

+5-54
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ protected override void Append(LoggingEvent loggingEvent)
200200
loggingEvent.EnsureNotNull();
201201
if (_consoleOutputWriter is not null)
202202
{
203-
IntPtr consoleHandle = GetStdHandle(_writeToErrorStream ? StdErrorHandle : StdOutputHandle);
203+
IntPtr consoleHandle = NativeMethods.GetStdHandle(_writeToErrorStream ? NativeMethods.StdErrorHandle : NativeMethods.StdOutputHandle);
204204

205205
// Default to white on black
206206
ushort colorInfo = (ushort)Colors.White;
@@ -215,10 +215,10 @@ protected override void Append(LoggingEvent loggingEvent)
215215
string strLoggingMessage = RenderLoggingEvent(loggingEvent);
216216

217217
// get the current console color - to restore later
218-
GetConsoleScreenBufferInfo(consoleHandle, out ConsoleScreenBufferInfo bufferInfo);
218+
NativeMethods.GetConsoleScreenBufferInfo(consoleHandle, out NativeMethods.ConsoleScreenBufferInfo bufferInfo);
219219

220220
// set the console colors
221-
SetConsoleTextAttribute(consoleHandle, colorInfo);
221+
NativeMethods.SetConsoleTextAttribute(consoleHandle, colorInfo);
222222

223223
// Using WriteConsoleW seems to be unreliable.
224224
// If a large buffer is written, say 15,000 chars
@@ -315,7 +315,7 @@ protected override void Append(LoggingEvent loggingEvent)
315315
_consoleOutputWriter.Write(messageCharArray, 0, arrayLength);
316316

317317
// Restore the console back to its previous color scheme
318-
SetConsoleTextAttribute(consoleHandle, bufferInfo.wAttributes);
318+
NativeMethods.SetConsoleTextAttribute(consoleHandle, bufferInfo.wAttributes);
319319

320320
if (appendNewline)
321321
{
@@ -344,7 +344,7 @@ public override void ActivateOptions()
344344
Stream consoleOutputStream = _writeToErrorStream ? Console.OpenStandardError() : Console.OpenStandardOutput();
345345

346346
// Look up the codepage encoding for the console
347-
Encoding consoleEncoding = EncodingWithoutPreamble.Get(Encoding.GetEncoding(GetConsoleOutputCP()));
347+
Encoding consoleEncoding = EncodingWithoutPreamble.Get(Encoding.GetEncoding(NativeMethods.GetConsoleOutputCP()));
348348

349349
// Create a writer around the console stream
350350
_consoleOutputWriter = new StreamWriter(consoleOutputStream, consoleEncoding, 0x100)
@@ -391,55 +391,6 @@ public override void ActivateOptions()
391391
/// </remarks>
392392
private StreamWriter? _consoleOutputWriter;
393393

394-
[DllImport("Kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
395-
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
396-
private static extern int GetConsoleOutputCP();
397-
398-
[DllImport("Kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
399-
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
400-
private static extern bool SetConsoleTextAttribute(
401-
IntPtr consoleHandle,
402-
ushort attributes);
403-
404-
[DllImport("Kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
405-
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
406-
private static extern bool GetConsoleScreenBufferInfo(
407-
IntPtr consoleHandle,
408-
out ConsoleScreenBufferInfo bufferInfo);
409-
410-
private const uint StdOutputHandle = unchecked((uint)-11);
411-
private const uint StdErrorHandle = unchecked((uint)-12);
412-
413-
[DllImport("Kernel32.dll", SetLastError = true, CharSet = CharSet.Auto)]
414-
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
415-
private static extern IntPtr GetStdHandle(uint type);
416-
417-
[StructLayout(LayoutKind.Sequential)]
418-
private struct Coord
419-
{
420-
public ushort x;
421-
public ushort y;
422-
}
423-
424-
[StructLayout(LayoutKind.Sequential)]
425-
private struct SmallRect
426-
{
427-
public ushort Left;
428-
public ushort Top;
429-
public ushort Right;
430-
public ushort Bottom;
431-
}
432-
433-
[StructLayout(LayoutKind.Sequential)]
434-
private struct ConsoleScreenBufferInfo
435-
{
436-
public Coord dwSize;
437-
public Coord dwCursorPosition;
438-
public ushort wAttributes;
439-
public SmallRect srWindow;
440-
public Coord dwMaximumWindowSize;
441-
}
442-
443394
/// <summary>
444395
/// A class to act as a mapping between the level that a logging call is made at and
445396
/// the color it should be displayed as.

src/log4net/Appender/FileAppender.cs

+16-18
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ namespace log4net.Appender;
3535
/// </summary>
3636
/// <remarks>
3737
/// <para>
38-
/// Logging events are sent to the file specified by
39-
/// the <see cref="File"/> property.
38+
/// Logging events are sent to the file specified by the <see cref="File"/> property.
4039
/// </para>
4140
/// <para>
4241
/// The file can be opened in either append or overwrite mode
@@ -90,23 +89,22 @@ private sealed class LockingStream(LockingModelBase lockingModel) : Stream, IDis
9089
public sealed class LockStateException : LogException
9190
{
9291
public LockStateException(string message)
93-
: base(message)
94-
{
95-
}
92+
: base(message)
93+
{ }
9694

9795
public LockStateException()
98-
{
99-
}
96+
{ }
10097

101-
public LockStateException(string message, Exception innerException) : base(message, innerException)
102-
{
103-
}
98+
public LockStateException(string message, Exception innerException)
99+
: base(message, innerException)
100+
{ }
104101

105-
private LockStateException(SerializationInfo info, StreamingContext context) : base(info, context)
106-
{
107-
}
102+
private LockStateException(SerializationInfo info, StreamingContext context)
103+
: base(info, context)
104+
{ }
108105
}
109106

107+
private readonly object syncRoot = new();
110108
[System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2213:Disposable fields should be disposed", Justification = "todo")]
111109
private Stream? _realStream;
112110
private int _lockLevel;
@@ -173,7 +171,7 @@ private Stream AssertLocked()
173171
public bool AcquireLock()
174172
{
175173
bool ret = false;
176-
lock (this)
174+
lock (syncRoot)
177175
{
178176
if (_lockLevel == 0)
179177
{
@@ -193,7 +191,7 @@ public bool AcquireLock()
193191

194192
public void ReleaseLock()
195193
{
196-
lock (this)
194+
lock (syncRoot)
197195
{
198196
_lockLevel--;
199197
if (_lockLevel == 0)
@@ -843,8 +841,8 @@ public override void OnClose()
843841
/// Specify default locking model
844842
/// </summary>
845843
/// <typeparam name="TLockingModel">Type of LockingModel</typeparam>
846-
public static void SetDefaultLockingModelType<TLockingModel>()
847-
where TLockingModel : LockingModelBase
844+
public static void SetDefaultLockingModelType<TLockingModel>()
845+
where TLockingModel : LockingModelBase
848846
=> _defaultLockingModelType = typeof(TLockingModel);
849847

850848
/// <summary>
@@ -1215,7 +1213,7 @@ protected virtual void OpenFile(string fileName, bool append)
12151213
}
12161214
}
12171215

1218-
lock (this)
1216+
lock (SyncRoot)
12191217
{
12201218
Reset();
12211219

src/log4net/Appender/LocalSyslogAppender.cs

+3-30
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ public override void ActivateOptions()
314314
_handleToIdentity = Marshal.StringToHGlobalAnsi(identString);
315315

316316
// open syslog
317-
openlog(_handleToIdentity, 1, Facility);
317+
NativeMethods.openlog(_handleToIdentity, 1, Facility);
318318
}
319319

320320
/// <summary>
@@ -338,7 +338,7 @@ protected override void Append(LoggingEvent loggingEvent)
338338

339339
// Call the local libc syslog method
340340
// The second argument is a printf style format string
341-
syslog(priority, "%s", message);
341+
NativeMethods.syslog(priority, "%s", message);
342342
}
343343

344344
/// <summary>
@@ -357,7 +357,7 @@ protected override void OnClose()
357357
try
358358
{
359359
// close syslog
360-
closelog();
360+
NativeMethods.closelog();
361361
}
362362
catch (DllNotFoundException)
363363
{
@@ -446,33 +446,6 @@ private static int GeneratePriority(SyslogFacility facility, SyslogSeverity seve
446446
/// </summary>
447447
private readonly LevelMapping _levelMapping = new();
448448

449-
/// <summary>
450-
/// Open connection to system logger.
451-
/// </summary>
452-
[DllImport("libc")]
453-
private static extern void openlog(IntPtr ident, int option, SyslogFacility facility);
454-
455-
/// <summary>
456-
/// Generate a log message.
457-
/// </summary>
458-
/// <remarks>
459-
/// <para>
460-
/// The libc syslog method takes a format string and a variable argument list similar
461-
/// to the classic printf function. As this type of vararg list is not supported
462-
/// by C# we need to specify the arguments explicitly. Here we have specified the
463-
/// format string with a single message argument. The caller must set the format
464-
/// string to <c>"%s"</c>.
465-
/// </para>
466-
/// </remarks>
467-
[DllImport("libc", CharSet = CharSet.Ansi, CallingConvention = CallingConvention.Cdecl)]
468-
private static extern void syslog(int priority, string format, string message);
469-
470-
/// <summary>
471-
/// Close descriptor used to write to system logger.
472-
/// </summary>
473-
[DllImport("libc")]
474-
private static extern void closelog();
475-
476449
/// <summary>
477450
/// A class to act as a mapping between the level that a logging call is made at and
478451
/// the syslog severity that is should be logged at.

src/log4net/Appender/OutputDebugStringAppender.cs

+2-13
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
using System.Runtime.InteropServices;
2121

2222
using log4net.Core;
23+
using log4net.Util;
2324

2425
namespace log4net.Appender;
2526

@@ -45,23 +46,11 @@ protected override void Append(LoggingEvent loggingEvent)
4546
}
4647
#endif
4748

48-
OutputDebugString(RenderLoggingEvent(loggingEvent));
49+
NativeMethods.OutputDebugString(RenderLoggingEvent(loggingEvent));
4950
}
5051

5152
/// <summary>
5253
/// This appender requires a <see cref="Layout"/> to be set.
5354
/// </summary>
5455
protected override bool RequiresLayout => true;
55-
56-
/// <summary>
57-
/// Stub for OutputDebugString native method
58-
/// </summary>
59-
/// <param name="message">the string to output</param>
60-
#if NETSTANDARD2_0_OR_GREATER
61-
[DllImport("CoreDll.dll")]
62-
#else
63-
[DllImport("Kernel32.dll")]
64-
#endif
65-
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
66-
protected static extern void OutputDebugString(string message);
6756
}

src/log4net/Appender/RollingFileAppender.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ protected virtual void AdjustFileBeforeAppend()
564564
/// </remarks>
565565
protected override void OpenFile(string fileName, bool append)
566566
{
567-
lock (this)
567+
lock (SyncRoot)
568568
{
569569
fileName = GetNextOutputFileName(fileName);
570570

0 commit comments

Comments
 (0)