Repository navigation
Simplify inherited IDisposable logic due to Serilog #1477
Description
Activity
- addedIssue-EnhancementA feature request (enhancement).A feature request (enhancement).
on May 13, 2021 - removedNeeds: TriageMaintainer attention needed!Maintainer attention needed!
on May 18, 2021 When I run Start-EditorServices from my application, I get the following error:
ERROR: Exception encountered starting EditorServices. Exception logged in D:\a\1\s\src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs on line 246 in EndProcessing: System.MissingMethodException: Method not found: 'Microsoft.Extensions.Logging.ILoggerFactory Serilog.SerilogLoggerFactoryExtensions.AddSerilog(Microsoft.Extensions.Logging.ILoggerFactory, Serilog.ILogger, Boolean)'. at Microsoft.PowerShell.EditorServices.Hosting.EditorServicesServerFactory.Create(String logPath, Int32 minimumLogLevel, IObservable`1 hostLogger) at Microsoft.PowerShell.EditorServices.Hosting.EditorServicesRunner..ctor(HostLogger logger, EditorServicesConfig config, ISessionFileWriter sessionFileWriter, IReadOnlyCollection`1 loggersToUnsubscribe) in D:\a\1\s\src\PowerShellEditorServices.Hosting\Internal\EditorServicesRunner.cs:line 45 at Microsoft.PowerShell.EditorServices.Hosting.EditorServicesLoader.LoadAndRunEditorServicesAsync() in D:\a\1\s\src\PowerShellEditorServices.Hosting\EditorServicesLoader.cs:line 204 at Microsoft.PowerShell.EditorServices.Commands.StartEditorServicesCommand.EndProcessing() in D:\a\1\s\src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs:line 241I’ve added the same 4 Serilog Nuget packages, including serilog.extensions.logging and it still happens.
The only workaround is to add the PowerShellEditorServices csproj to my Solution, and then add it as a Project Reference. If I add it to my solution but don’t add it as a project reference, the error occurs.
For context, this has been happening for at least a year, but I forgot about it until this week when changes were made which should allow me to use the production build of PSES.
I don’t know if the proposed changes here would solve my issue, but I didn’t want to make a new issue when there was already one discussing Serilog.
- ghost addedNeeds: Maintainer AttentionMaintainer attention needed!Maintainer attention needed!
on Jun 17, 2021 Thank you, at least I'm not crazy or the only one with this issue.
Serilog is not in my GAC but it is being used in the calling application. I suspect that is the issue.
Reacted by Andy Jordanandyleejordan commented
on Jun 17, 2021 MemberAuthorMore actionsPerhaps we should prioritize dumping Serilog 🤔
Perhaps we should prioritize dumping Serilog 🤔
I probably could have removed it in less time than I've spent debugging this.
Interestingly enough, somebody just put a Pull Request into Serilog.Extensions.Logging an hour ago that may address this issue.
What if the problem is that this particular interface doesn't get exposed when called from a parent process running an incompatible .net core version, in my case .net 5.0
I removed all references to Serilog and am now getting
DEBUG: Loaded into load context "PsesLoadContext" Microsoft.PowerShell.EditorServices.Hosting.PsesLoadContext #1: Microsoft.Extensions.DependencyInjection.Abstractions, Version=5.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60 DEBUG: Loader disposed ERROR: Exception encountered starting EditorServices. Exception logged in C:\Users\dkattan\Source\Repos\PowerShellEditorServices\src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs on line 246 in EndProcessing: System.EntryPointNotFoundException: Entry point was not found. at System.Collections.Generic.IEnumerable`1.GetEnumerator() at System.Linq.Enumerable.Any[TSource](IEnumerable`1 source, Func`2 predicate) at Microsoft.Extensions.DependencyInjection.Extensions.ServiceCollectionDescriptorExtensions.TryAdd(IServiceCollection collection, ServiceDescriptor descriptor) at Microsoft.Extensions.DependencyInjection.OptionsServiceCollectionExtensions.AddOptions(IServiceCollection services) at OmniSharp.Extensions.JsonRpc.JsonRpcOptionsRegistryBase`1..ctor() at OmniSharp.Extensions.JsonRpc.JsonRpcServerOptionsBase`1..ctor() at OmniSharp.Extensions.LanguageServer.Protocol.LanguageProtocolRpcOptionsBase`1..ctor() at OmniSharp.Extensions.LanguageServer.Server.LanguageServerOptions..ctor() at OmniSharp.Extensions.LanguageServer.Server.LanguageServer.From(Action`1 optionsAction, IServiceProvider outerServiceProvider, CancellationToken cancellationToken) at OmniSharp.Extensions.LanguageServer.Server.LanguageServer.From(Action`1 optionsAction) at Microsoft.PowerShell.EditorServices.Server.PsesLanguageServer.StartAsync() in C:\Users\dkattan\Source\Repos\PowerShellEditorServices\src\PowerShellEditorServices\Server\PsesLanguageServer.cs:line 69 at Microsoft.PowerShell.EditorServices.Hosting.EditorServicesRunner.CreateEditorServicesAndRunUntilShutdown() in C:\Users\dkattan\Source\Repos\PowerShellEditorServices\src\PowerShellEditorServices.Hosting\Internal\EditorServicesRunner.cs:line 183 at Microsoft.PowerShell.EditorServices.Commands.StartEditorServicesCommand.EndProcessing() in C:\Users\dkattan\Source\Repos\PowerShellEditorServices\src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs:line 241Digging deeper, I think the overall problem might be the version of Serilog and Microsoft.Extensions.Logging that Omnisharp is using:
https://github.com/OmniSharp/csharp-language-server-protocol/blob/231a53f5e6f59fdca88478ed3f44f832d68491c1/Directory.Build.targets#L20andyleejordan commented
on Jun 18, 2021 MemberAuthorMore actionsHm very interesting, especially since we apparently use vastly different versions:
PowerShellEditorServices/src/PowerShellEditorServices/PowerShellEditorServices.csproj
Line 31 in da481f2
<PackageReference Include="Microsoft.Extensions.Logging" Version="5.0.0" /> PowerShellEditorServices/src/PowerShellEditorServices/PowerShellEditorServices.csproj
Line 36 in da481f2
<PackageReference Include="Serilog.Extensions.Logging" Version="3.0.1" /> andyleejordan commented
on Jun 21, 2021 MemberAuthorMore actionsI probably could have removed it in less time than I've spent debugging this.
I would 💯accept a PR replacing Serilog Darren Kattan (@dkattan), looking into it myself.
Open to helping resolve this if it turns out to be worthwhile - a bit swamped on the Serilog side of things, so would need some tactical PRs that don't introduce a huge amount of churn (which makes the one Darren Kattan (@dkattan) linked above a bit tricky to move forward with in a hurry).
Unfortunately, with Serilog right down the bottom of the dependency graph, this stuff is never fun :-) ... good luck, either way! 👍
Reacted by Andy JordanI got it to work!
I added the following NuGet packages to my application
- Microsoft.Extensions.Logging.Abstractions Version=5.0.0
- OmniSharp.Extensions.DebugAdapter.Server Version=0.19.2
- OmniSharp.Extensions.LanguageServer Version=0.19.2
- Serilog.Sinks.Async Version=1.5.0
- System.IO.FileSystem.Primitives Version=4.3.0
Then deleted everything in module\PowerShellEditorServices\bin\Common except Microsoft.PowerShell.EditorServices.dll and magically everything worked.
I believe the problem here to be here
We aren't checking the loaded assemblies to verify that we actually need to load the files before loading them.
The tip off was me watching the ordering of the loaded assemblies when it worked versus when it didn't.
When it worked it loaded in this order
- Microsoft.PowerShell.EditorServices.dll
- System.IO.FileSystem.Primitives.dll
- OmniSharp.Extensions.LanguageServer.dll
- Nerdbank.Streams.dll
- OmniSharp.Extensions.JsonRpc.dll
- MediatR.dll
- OmniSharp.Extensions.LanguageProtocol.dll
- System.Resources.ResourceManager.dll
- Microsoft.Bcl.AsyncInterfaces.dll
- System.Reactive.dll
- System.Threading.Timer.dll
- OmniSharp.Extensions.LanguageServer.Shared.dll
- Microsoft.PowerShell.PSReadLine2.dll
When it didn't
- Microsoft.PowerShell.EditorServices.dll
- Microsoft.Extensions.Logging.Abstractions.dll <-- Already loaded
- Microsoft.Extensions.Logging.dll <-- Already loaded
It wasn't that it was loading the incorrect version, the versions were identical, I think it has to do with my calling context having already loaded Serilog and a variety of other assemblies, and PSES attempting to load them again.
I think if I modify the aforementioned code to check if the assembly is already loaded before calling LoadFromAssemblyPath, we'll be good.
Reacted by Nicholas Blumhardt and Andy Jordan- removedNeeds: Maintainer AttentionMaintainer attention needed!Maintainer attention needed!
on Jul 20, 2021 andyleejordan commented
on Nov 15, 2024 MemberAuthorMore actionsSerilog is now gone! I think we left the classes as
IDisposablebut Justin Grote (@JustinGrote) if that's not needed any more, we can get to this now.- linked a pull request that will close this issueSimplify inherited IDisposable logic due to Serilog #2335
on Aug 6, 2026
Noted in #1474, a bunch of our factories used to abstract our logging setup during initialization are
IDisposableand so have to call each other'sDispose()methods, all due to the fact that we need to callSerilog.Log.CloseAndFlush()at the end of the program. This is actually a place where referencing a global is perhaps a better pattern, since the logger is already global, but Serilog provides an even better option by passing the parameterdispose=truewhen usingSerilogLoggerFactoryExtensions.AddSerilog(dispose=true).