diff --git a/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstance.cs b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstance.cs index 872631db4..e3c9317aa 100644 --- a/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstance.cs +++ b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstance.cs @@ -1,4 +1,4 @@ -using System.Diagnostics.CodeAnalysis; +using System.Diagnostics.Contracts; using Microsoft.Extensions.Logging; using Rhino; using Rhino.Runtime.InProcess; @@ -9,55 +9,34 @@ namespace Speckle.Importers.Rhino.Internal; -internal sealed class ImporterInstance(Sender sender, ILogger logger) : IDisposable +internal sealed class ImporterInstance(ImporterArgs args, Sender sender, ILogger logger) : IDisposable { - private readonly ILogger _logger = logger; private readonly RhinoCore _rhinoInstance = new(["/netcore-8"], WindowStyle.NoWindow); - private RhinoDoc? _rhinoDoc; + private readonly RhinoDoc _rhinoDoc = OpenDocument(args, logger); - public async Task Run(ImporterArgs args, CancellationToken cancellationToken) - { - using var scopeJobId = ActivityScope.SetTag("jobId", args.JobId); - // using var scopeJobType = ActivityScope.SetTag("jobType", a.JobType); - using var scopeAttempt = ActivityScope.SetTag("job.attempt", args.Attempt.ToString()); - using var scopeServerUrl = ActivityScope.SetTag("serverUrl", args.Account.serverInfo.url); - using var scopeProjectId = ActivityScope.SetTag("projectId", args.Project.id); - using var scopeModelId = ActivityScope.SetTag("modelId", args.ModelId); - using var scopeBlobId = ActivityScope.SetTag("blobId", args.BlobId); - using var scopeFileType = ActivityScope.SetTag("fileType", Path.GetExtension(args.FilePath).TrimStart('.')); - UserActivityScope.AddUserScope(args.Account); - - var result = await TryImport(args, cancellationToken); - return result; - } + private readonly IReadOnlyList _scopes = + [ + ActivityScope.SetTag("jobId", args.JobId), + ActivityScope.SetTag("job.attempt", args.Attempt.ToString()), + // ActivityScope.SetTag("jobType", args.JobType), + ActivityScope.SetTag("serverUrl", args.Account.serverInfo.url), + ActivityScope.SetTag("projectId", args.Project.id), + ActivityScope.SetTag("modelId", args.ModelId), + ActivityScope.SetTag("blobId", args.BlobId), + ActivityScope.SetTag("fileType", Path.GetExtension(args.FilePath).TrimStart('.')), + UserActivityScope.AddUserScope(args.Account), + ]; - [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "IPC")] - private async Task TryImport(ImporterArgs args, CancellationToken cancellationToken) + public async Task RunRhinoImport(CancellationToken cancellationToken) { try { - var version = await RunRhinoImport(args, cancellationToken); - return new ImporterResponse { Version = version, ErrorMessage = null }; - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Import attempt failed with exception"); - return new ImporterResponse { ErrorMessage = ex.Message, Version = null }; - } - } - - private async Task RunRhinoImport(ImporterArgs args, CancellationToken cancellationToken) - { - try - { - using var config = GetConfig(Path.GetExtension(args.FilePath)); - logger.LogInformation("Opening file {FilePath}", args.FilePath); - - _rhinoDoc = config.OpenInHeadlessDocument(args.FilePath); RhinoDoc.ActiveDoc = _rhinoDoc; - var version = await sender.Send(args.Project, args.ModelId, args.Account, cancellationToken); + var version = await sender + .Send(args.Project, args.ModelId, args.Account, cancellationToken) + .ConfigureAwait(false); return version; } finally @@ -66,6 +45,14 @@ private async Task RunRhinoImport(ImporterArgs args, CancellationToken } } + private static RhinoDoc OpenDocument(ImporterArgs args, ILogger logger) + { + using var config = GetConfig(Path.GetExtension(args.FilePath)); + logger.LogInformation("Opening file {FilePath}", args.FilePath); + return config.OpenInHeadlessDocument(args.FilePath); + } + + [Pure] private static IFileTypeConfig GetConfig(string extension) => extension.ToLowerInvariant() switch { @@ -83,5 +70,9 @@ public void Dispose() // https://discourse.mcneel.com/t/rhino-inside-fatal-app-crashes-when-disposing-headless-documents/208673 _rhinoDoc?.Dispose(); _rhinoInstance.Dispose(); + foreach (var scope in _scopes) + { + scope.Dispose(); + } } } diff --git a/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstanceFactory.cs b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstanceFactory.cs new file mode 100644 index 000000000..b86d58fb4 --- /dev/null +++ b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ImporterInstanceFactory.cs @@ -0,0 +1,8 @@ +using Microsoft.Extensions.Logging; + +namespace Speckle.Importers.Rhino.Internal; + +internal sealed class ImporterInstanceFactory(Sender sender, ILogger logger) +{ + public ImporterInstance Create(ImporterArgs args) => new(args, sender, logger); +} diff --git a/Importers/Rhino/Speckle.Importers.Rhino/Internal/ServiceRegistration.cs b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ServiceRegistration.cs index bf40db501..a1e1440be 100644 --- a/Importers/Rhino/Speckle.Importers.Rhino/Internal/ServiceRegistration.cs +++ b/Importers/Rhino/Speckle.Importers.Rhino/Internal/ServiceRegistration.cs @@ -20,6 +20,7 @@ public static IServiceCollection AddRhinoImporter(this IServiceCollection servic services.AddTransient(); services.AddTransient(); services.AddTransient(); + services.AddTransient(); // override default thread context services.AddSingleton(new ImporterThreadContext()); diff --git a/Importers/Rhino/Speckle.Importers.Rhino/Program.cs b/Importers/Rhino/Speckle.Importers.Rhino/Program.cs index c4aa6cdb8..c6a9fe7e0 100644 --- a/Importers/Rhino/Speckle.Importers.Rhino/Program.cs +++ b/Importers/Rhino/Speckle.Importers.Rhino/Program.cs @@ -1,9 +1,11 @@ -using System.Text.Json; +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using RhinoInside; using Speckle.Importers.Rhino.Internal; +using Version = Speckle.Sdk.Api.GraphQL.Models.Version; namespace Speckle.Importers.Rhino; @@ -18,9 +20,12 @@ static Program() Console.WriteLine($"Loading Rhino @ {Resolver.RhinoSystemDirectory}"); } + [SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "IPC")] public static async Task Main(string[] args) { ILogger? logger = null; + ImporterInstance? importer = null; + try { var importerArgs = JsonSerializer.Deserialize(args[0], s_serializerOptions); @@ -32,13 +37,34 @@ public static async Task Main(string[] args) TaskScheduler.UnobservedTaskException += (_, eventArgs) => logger.LogCritical(eventArgs.Exception, "Unobserved Task Exception"); - var importer = serviceProvider.GetRequiredService(); + var factory = serviceProvider.GetRequiredService(); + + // Error handling flow below here looks a bit of a mess, but we're having to navigate threading issues with rhino inside + // https://discourse.mcneel.com/t/rhino-inside-fatal-app-crashes-when-disposing-headless-documents/208673/7 + try + { + // This needs to be called on the main thread + importer = factory.Create(importerArgs); + } + catch (Exception ex) + { + WriteResult(new() { ErrorMessage = ex.Message }, importerArgs.ResultsPath); + return; + } + // As soon as the main thread is yielded, it will be hogged by Rhino + // Task.Run ensures we run everything on a thread pool thread. await Task.Run(async () => { - var result = await importer.Run(importerArgs, CancellationToken.None); - var serializedResult = JsonSerializer.Serialize(result, s_serializerOptions); - File.WriteAllLines(importerArgs.ResultsPath, [serializedResult]); + try + { + Version result = await importer.RunRhinoImport(CancellationToken.None).ConfigureAwait(false); + WriteResult(new() { Version = result }, importerArgs.ResultsPath); + } + catch (Exception ex) + { + WriteResult(new() { ErrorMessage = ex.Message }, importerArgs.ResultsPath); + } }) .ConfigureAwait(false); } @@ -53,7 +79,18 @@ await Task.Run(async () => { Console.WriteLine(MESSAGE); } + throw; } + finally + { + importer?.Dispose(); + } + } + + private static void WriteResult(ImporterResponse result, string resultsPath) + { + var serializedResult = JsonSerializer.Serialize(result, s_serializerOptions); + File.WriteAllLines(resultsPath, [serializedResult]); } }