Should we dispose EventLog if there was an Interop.Errors.ERROR_INVALID_PARAMETER?

[omajid]

runtime/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLog.cs

Lines 651 to 664 in 9317db0

	EventLog log = new EventLog(logNames[i], machineName);
	SafeEventLogReadHandle handle = Interop.Advapi32.OpenEventLog(machineName, logNames[i]);
	if (!handle.IsInvalid)
	{
	handle.Close();
	logs.Add(log);
	}
	else if (Marshal.GetLastWin32Error() != Interop.Errors.ERROR_INVALID_PARAMETER)
	{
	// This api should return the list of all event logs present on the system even if the current user can't open the log.
	// Windows returns ERROR_INVALID_PARAMETER for special keys which were added in RS5+ but do not represent actual event logs.
	logs.Add(log);
	}

In the two conditions, we save the just-created log object. Should we call log.Dispose() if neither of those branches are taken?

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-eventlog
See info in area-owners.md if you want to be subscribed.

[ericstj]

If we create a disposable object and don't return it, then we should dispose it. If we still return it, then it should be the caller's responsibility to dispose.

Here we create it but don't expose it, so it could be disposed. Perhaps the original authors here "knew" that no disposable resources would be created.

I don't see any disposable resources created in

runtime/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLogInternal.cs

Lines 112 to 129 in 351926a

	public EventLogInternal(string logName, string machineName, string source, EventLog parent)
	{
	ArgumentNullException.ThrowIfNull(logName);
	if (!ValidLogName(logName, true))
	throw new ArgumentException(SR.BadLogName);
	if (!SyntaxCheck.CheckMachineName(machineName))
	throw new ArgumentException(SR.Format(SR.InvalidParameter, nameof(machineName), machineName));
	this.machineName = machineName;
	this.logName = logName;
	this.sourceName = source;
	readHandle = null;
	writeHandle = null;
	boolFlags[Flag_forwards] = true;
	this.parent = parent;
	}

So probably this isn't an actual leak, but could be updated. @omajid did you notice any actual leak here?

[omajid]

did you notice any actual leak here?

No. It was flagged by a code scanner.