diff --git a/docs/OPEN_QUESTIONS.md b/docs/OPEN_QUESTIONS.md index e9c8812..c221ef7 100644 --- a/docs/OPEN_QUESTIONS.md +++ b/docs/OPEN_QUESTIONS.md @@ -29,6 +29,30 @@ --- +## Само-ревью кода (ночная сессия) — итоги + +**Исправлено сразу:** +- C1/C2 — утечки COM и устаревшие id: `PartModeler` освобождает RCW и сбрасывает реестр + (`ResetAsync`/`Dispose`), `DocumentTools` сбрасывает реестр при create/open/close; + `DocumentService` освобождает транзитные `IDocuments`/`IKompasDocument`. +- M3 — `KompasSession.Dispose`: ограниченное ожидание (5 c) вместо вечного блока. +- M4 — `ExtrudeAsync`: убран `dynamic`, конкретные `ksBossExtrusionDefinition`/ + `ksCutExtrusionDefinition`; `throughAll` разрешён только для выреза. +- m7 — `extrude_cut`: дефолт `throughAll=false`, без молчаливой подмены `depth`. +- Регрессионный тест: после `ResetAsync` старый id эскиза недействителен. + +**⚠️ Отложено на проработку (из ревью):** +- M5 — отмена/насос сообщений: `CancellationToken` не прерывает уже идущий COM-вызов; зависший + вызов (модальный диалог КОМПАС) блокирует всю очередь. Возможное решение — диспетчер с насосом + сообщений + подавление диалогов. Связано с [конкурентностью MCP]. +- m6 — `colorBPP=24` для снимка: для GIF (палитра) может не подойти; снимок по умолчанию PNG. +- m8 — приведения enum к `short` для `GetPart/NewEntity/GetDefaultEntity`: на v24 работает + (тесты зелёные), но стоит сверить точные типы параметров в interop. +- m10 — `_app/_kompas` читаются из `IsConnected/Status()` вне STA-потока без `volatile` + (доброкачественное устаревшее чтение). +- m11 — трансляция `COMException`/HRESULT в понятные сообщения (сейчас понятны только наши + `InvalidOperationException`); добавить текст последней ошибки КОМПАС. + ## Журнал решений по ходу (дополняется автоматически во время работы) diff --git a/src/Kompas.Mcp.Core/Documents/DocumentService.cs b/src/Kompas.Mcp.Core/Documents/DocumentService.cs index b3311bf..9eba22e 100644 --- a/src/Kompas.Mcp.Core/Documents/DocumentService.cs +++ b/src/Kompas.Mcp.Core/Documents/DocumentService.cs @@ -1,3 +1,4 @@ +using System.Runtime.InteropServices; using System.Runtime.Versioning; using Kompas.Mcp.Core.Threading; using Kompas6Constants; @@ -22,18 +23,22 @@ public sealed class DocumentService public Task CreateAsync(KompasDocumentType type, bool visible = true, CancellationToken ct = default) => _dispatcher.InvokeAsync(() => { - var doc = (IKompasDocument)_session.Application.Documents.Add(DocumentTypes.ToEnum(type), visible); - return ToInfo(doc); + var docs = _session.Application.Documents; + var doc = (IKompasDocument)docs.Add(DocumentTypes.ToEnum(type), visible); + try { return ToInfo(doc); } + finally { ReleaseCom(doc); ReleaseCom(docs); } }, ct); /// Открыть документ с диска. public Task OpenAsync(string path, bool readOnly = false, bool visible = true, CancellationToken ct = default) => _dispatcher.InvokeAsync(() => { - var doc = (IKompasDocument)_session.Application.Documents.Open(path, visible, readOnly); + var docs = _session.Application.Documents; + var doc = (IKompasDocument)docs.Open(path, visible, readOnly); if (doc is null) throw new InvalidOperationException($"Не удалось открыть документ: {path}"); - return ToInfo(doc); + try { return ToInfo(doc); } + finally { ReleaseCom(doc); ReleaseCom(docs); } }, ct); /// Сводка по активному документу или null, если документов нет. @@ -41,25 +46,34 @@ public sealed class DocumentService => _dispatcher.InvokeAsync(() => { var doc = _session.Application.ActiveDocument; - return doc is null ? null : ToInfo(doc); + if (doc is null) return null; + try { return ToInfo(doc); } + finally { ReleaseCom(doc); } }, ct); /// Сохранить активный документ (для уже сохранённого — без диалога). public Task SaveAsync(CancellationToken ct = default) - => _dispatcher.InvokeAsync(() => RequireActive().Save(), ct); + => _dispatcher.InvokeAsync(() => WithActive(d => d.Save()), ct); /// Сохранить активный документ по указанному пути (безопасно для новых документов). public Task SaveAsAsync(string path, CancellationToken ct = default) - => _dispatcher.InvokeAsync(() => RequireActive().SaveAs(path), ct); + => _dispatcher.InvokeAsync(() => WithActive(d => d.SaveAs(path)), ct); /// Закрыть активный документ. — сохранять ли изменения. public Task CloseAsync(bool save = false, CancellationToken ct = default) - => _dispatcher.InvokeAsync(() => - RequireActive().Close(save ? DocumentCloseOptions.kdSaveChanges : DocumentCloseOptions.kdDoNotSaveChanges), ct); + => _dispatcher.InvokeAsync(() => WithActive(d => + d.Close(save ? DocumentCloseOptions.kdSaveChanges : DocumentCloseOptions.kdDoNotSaveChanges)), ct); - private IKompasDocument RequireActive() - => _session.Application.ActiveDocument - ?? throw new InvalidOperationException("Нет активного документа."); + private void WithActive(Action action) + => WithActive(d => { action(d); return true; }); + + private T WithActive(Func func) + { + var doc = _session.Application.ActiveDocument + ?? throw new InvalidOperationException("Нет активного документа."); + try { return func(doc); } + finally { ReleaseCom(doc); } + } private static DocumentInfo ToInfo(IKompasDocument doc) => new() { @@ -67,4 +81,13 @@ public sealed class DocumentService PathName = doc.PathName ?? string.Empty, Type = DocumentTypes.FromEnum(doc.DocumentType), }; + + private static void ReleaseCom(object? comObject) + { + if (comObject is not null && Marshal.IsComObject(comObject)) + { + try { Marshal.ReleaseComObject(comObject); } + catch { /* освобождение не критично */ } + } + } } diff --git a/src/Kompas.Mcp.Core/KompasSession.cs b/src/Kompas.Mcp.Core/KompasSession.cs index 1859048..b219a04 100644 --- a/src/Kompas.Mcp.Core/KompasSession.cs +++ b/src/Kompas.Mcp.Core/KompasSession.cs @@ -144,6 +144,7 @@ public sealed class KompasSession : IDisposable // Освобождаем COM-объекты на том же STA-потоке, на котором они созданы. try { + // Ограниченное ожидание: не виснем на старте остановки, если STA-поток занят COM-вызовом. _dispatcher.InvokeAsync(() => { // Quit не вызываем: не «убиваем» КОМПАС пользователя (см. docs/OPEN_QUESTIONS.md). @@ -151,11 +152,11 @@ public sealed class KompasSession : IDisposable if (_kompas is not null && Marshal.IsComObject(_kompas)) Marshal.ReleaseComObject(_kompas); _app = null; _kompas = null; - }).GetAwaiter().GetResult(); + }).Wait(TimeSpan.FromSeconds(5)); } - catch (ObjectDisposedException) + catch { - // Диспетчер уже остановлен — COM освободит GC-финализатор. + // Диспетчер остановлен / вызов завис — COM освободит GC-финализатор. } } } diff --git a/src/Kompas.Mcp.Core/Modeling/PartModeler.cs b/src/Kompas.Mcp.Core/Modeling/PartModeler.cs index 42540af..2673625 100644 --- a/src/Kompas.Mcp.Core/Modeling/PartModeler.cs +++ b/src/Kompas.Mcp.Core/Modeling/PartModeler.cs @@ -1,3 +1,4 @@ +using System.Runtime.InteropServices; using System.Runtime.Versioning; using Kompas.Mcp.Core.Threading; using Kompas6API5; @@ -11,7 +12,7 @@ namespace Kompas.Mcp.Core.Modeling; /// ссылаться на них в цикле «эскиз → операция → эскиз». /// [SupportedOSPlatform("windows")] -public sealed class PartModeler +public sealed class PartModeler : IDisposable { private const int MainLineStyle = 1; // системный стиль «Основная» @@ -122,23 +123,34 @@ public sealed class PartModeler var entity = part.NewEntity((short)type) as ksEntity ?? throw new InvalidOperationException("NewEntity операции выдавливания вернул null."); - // ksBossExtrusionDefinition и ksCutExtrusionDefinition имеют одинаковый набор - // нужных членов, но не общий интерфейс — настраиваем через dynamic. - dynamic def = entity.GetDefinition() - ?? throw new InvalidOperationException("GetDefinition() операции вернул null."); - - if (throughAll) + // Конкретные типы определений (без dynamic): ошибки связывания — на этапе компиляции. + if (cut) { - def.directionType = (short)Direction_Type.dtBoth; - def.SetSideParam(true, (short)End_Type.etThroughAll, depth, draftAngle, false); - def.SetSideParam(false, (short)End_Type.etThroughAll, depth, draftAngle, false); + var def = entity.GetDefinition() as ksCutExtrusionDefinition + ?? throw new InvalidOperationException("GetDefinition() выреза вернул не ksCutExtrusionDefinition."); + if (throughAll) + { + def.directionType = (short)Direction_Type.dtBoth; + def.SetSideParam(true, (short)End_Type.etThroughAll, depth, draftAngle, false); + def.SetSideParam(false, (short)End_Type.etThroughAll, depth, draftAngle, false); + } + else + { + def.directionType = (short)Direction_Type.dtNormal; + def.SetSideParam(forward, (short)End_Type.etBlind, depth, draftAngle, false); + } + def.SetSketch(sketch.Entity); } else { + if (throughAll) + throw new ArgumentException("throughAll поддерживается только для выреза (cut).", nameof(throughAll)); + var def = entity.GetDefinition() as ksBossExtrusionDefinition + ?? throw new InvalidOperationException("GetDefinition() выдавливания вернул не ksBossExtrusionDefinition."); def.directionType = (short)Direction_Type.dtNormal; def.SetSideParam(forward, (short)End_Type.etBlind, depth, draftAngle, false); + def.SetSketch(sketch.Entity); } - def.SetSketch(sketch.Entity); if (!entity.Create()) throw new InvalidOperationException("Create() операции выдавливания вернул FALSE."); @@ -156,6 +168,13 @@ public sealed class PartModeler doc3d.RebuildDocument(); }, ct); + /// + /// Сбросить реестр эскизов/операций и освободить их COM-объекты. Вызывать при смене + /// активного документа (создание/открытие/закрытие) — id больше не действительны. + /// + public Task ResetAsync(CancellationToken ct = default) + => _dispatcher.InvokeAsync(ResetCore, ct); + // ---- Внутреннее -------------------------------------------------------- private ksDocument3D ActiveDoc3D() @@ -182,6 +201,38 @@ public sealed class PartModeler { if (s.Editor is null) return; s.Definition.EndEdit(); + ReleaseCom(s.Editor); // редактор после EndEdit больше не нужен s.Editor = null; } + + private void ResetCore() + { + foreach (var s in _sketches.Values) + { + ReleaseCom(s.Editor); + ReleaseCom(s.Definition); + ReleaseCom(s.Entity); + } + foreach (var f in _features.Values) + ReleaseCom(f); + _sketches.Clear(); + _features.Clear(); + _nextId = 1; + } + + private static void ReleaseCom(object? comObject) + { + if (comObject is not null && Marshal.IsComObject(comObject)) + { + try { Marshal.ReleaseComObject(comObject); } + catch { /* освобождение не критично */ } + } + } + + public void Dispose() + { + // Освобождаем COM на STA-потоке; не виснем на старте, если поток занят. + try { _dispatcher.InvokeAsync(ResetCore).Wait(TimeSpan.FromSeconds(5)); } + catch { /* при остановке диспетчера COM освободит GC */ } + } } diff --git a/src/Kompas.Mcp.Host/Tools/DocumentTools.cs b/src/Kompas.Mcp.Host/Tools/DocumentTools.cs index 39f1bf7..3f20416 100644 --- a/src/Kompas.Mcp.Host/Tools/DocumentTools.cs +++ b/src/Kompas.Mcp.Host/Tools/DocumentTools.cs @@ -1,13 +1,14 @@ using System.ComponentModel; using Kompas.Mcp.Core; using Kompas.Mcp.Core.Documents; +using Kompas.Mcp.Core.Modeling; using ModelContextProtocol.Server; namespace Kompas.Mcp.Host.Tools; /// Инструменты управления документами. [McpServerToolType] -public sealed class DocumentTools(KompasSession session, DocumentService docs) +public sealed class DocumentTools(KompasSession session, DocumentService docs, PartModeler modeler) { [McpServerTool(Name = "document_create")] [Description("Создать новый документ и сделать его активным. type: part | assembly | drawing | fragment | specification | text.")] @@ -15,6 +16,7 @@ public sealed class DocumentTools(KompasSession session, DocumentService docs) { await session.ConnectAsync(); var info = await docs.CreateAsync(DocumentTypes.Parse(type)); + await modeler.ResetAsync(); // прежние id эскизов/операций недействительны return $"Создан документ типа {info.Type}."; } @@ -26,6 +28,7 @@ public sealed class DocumentTools(KompasSession session, DocumentService docs) { await session.ConnectAsync(); var info = await docs.OpenAsync(path, readOnly); + await modeler.ResetAsync(); return $"Открыт документ: {info.Name} ({info.Type})."; } @@ -53,6 +56,7 @@ public sealed class DocumentTools(KompasSession session, DocumentService docs) { await session.ConnectAsync(); var ok = await docs.CloseAsync(save); + await modeler.ResetAsync(); return ok ? "Документ закрыт." : "Не удалось закрыть документ."; } diff --git a/src/Kompas.Mcp.Host/Tools/FeatureTools.cs b/src/Kompas.Mcp.Host/Tools/FeatureTools.cs index fa372f0..6d6a27b 100644 --- a/src/Kompas.Mcp.Host/Tools/FeatureTools.cs +++ b/src/Kompas.Mcp.Host/Tools/FeatureTools.cs @@ -19,15 +19,15 @@ public sealed class FeatureTools(KompasSession session, PartModeler modeler) } [McpServerTool(Name = "extrude_cut")] - [Description("Вырезать выдавливанием по эскизу. throughAll=true — насквозь (для отверстий, depth игнорируется); иначе на глубину depth.")] + [Description("Вырезать выдавливанием по эскизу. Для сквозного отверстия передайте throughAll=true (depth не нужен). Иначе вырез на глубину depth (мм).")] public async Task ExtrudeCut( int sketchId, - [Description("Глубина, мм (если не throughAll)")] double depth = 0, - [Description("Вырезать насквозь")] bool throughAll = true, + [Description("Глубина, мм (нужна, если throughAll=false)")] double depth = 0, + [Description("Вырезать насквозь через всю деталь")] bool throughAll = false, [Description("Прямое направление")] bool forward = true) { await session.ConnectAsync(); - var id = await modeler.ExtrudeAsync(sketchId, depth <= 0 ? 1 : depth, cut: true, forward: forward, throughAll: throughAll); + var id = await modeler.ExtrudeAsync(sketchId, depth, cut: true, forward: forward, throughAll: throughAll); return $"Вырез (cut) создан, id={id}."; } diff --git a/tests/Kompas.Mcp.Tests/Integration/ModelingTests.cs b/tests/Kompas.Mcp.Tests/Integration/ModelingTests.cs index e786e95..0c9eb8e 100644 --- a/tests/Kompas.Mcp.Tests/Integration/ModelingTests.cs +++ b/tests/Kompas.Mcp.Tests/Integration/ModelingTests.cs @@ -52,4 +52,22 @@ public sealed class ModelingTests await _docs.CloseAsync(save: false); } } + + [Fact] + public async Task Reset_invalidates_sketch_ids() + { + await _docs.CreateAsync(KompasDocumentType.Part); + try + { + var s = await _modeler.OpenSketchAsync(BasePlane.XOY); + await _modeler.ResetAsync(); + // После сброса реестра старый id недействителен. + await Assert.ThrowsAsync( + () => _modeler.AddCircleAsync(s, 0, 0, 5)); + } + finally + { + await _docs.CloseAsync(save: false); + } + } }