Мелкие исправления потокобезопасности

src/OneScript.DebugServices/DefaultBreakpointManager.cs

@@ -5,6 +5,7 @@ This Source Code Form is subject to the terms of the
 at http://mozilla.org/MPL/2.0/.
 ----------------------------------------------------------*/
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using OneScript.Commons;
@@ -14,37 +15,49 @@ namespace OneScript.DebugServices
 {
     public class DefaultBreakpointManager : IBreakpointManager
     {
-        private readonly Dictionary<string, string> _exceptionBreakpointsFilters = new Dictionary<string, string>();
-        private readonly List<BreakpointDescriptor> _breakpoints = new List<BreakpointDescriptor>();
+        // Точки задает поток отладчика, а проверяют потоки скриптов: списки не меняются, а подменяются целиком
+        private volatile Dictionary<string, string> _exceptionBreakpointsFilters = new Dictionary<string, string>();
+        private volatile BreakpointDescriptor[] _breakpoints = Array.Empty<BreakpointDescriptor>();
+        private readonly object _lock = new object();
         private int _idsGenerator;
 
         public void SetExceptionBreakpoints((string Id, string Condition)[] filters)
         {
-            _exceptionBreakpointsFilters?.Clear();
-            filters?.ForEach(c =>_exceptionBreakpointsFilters.Add(c.Id, c.Condition));
+            var newFilters = new Dictionary<string, string>();
+            filters?.ForEach(c => newFilters.Add(c.Id, c.Condition));
+            _exceptionBreakpointsFilters = newFilters;
         }
 
         public void SetBreakpoints(string module, (int Line, string Condition)[] breakpoints)
         {
-            var cleaned = _breakpoints.Where(x => x.Module != module)
-                .ToList();
+            lock (_lock)
+            {
+                var cleaned = _breakpoints.Where(x => x.Module != module)
+                    .ToList();
 
-            var range = breakpoints.Select(x => new BreakpointDescriptor(_idsGenerator++) { LineNumber = x.Line, Module = module, Condition = x.Condition });
-            cleaned.AddRange(range);
-            _breakpoints.Clear();
-            _breakpoints.AddRange(cleaned);
+                var range = breakpoints.Select(x => new BreakpointDescriptor(_idsGenerator++) { LineNumber = x.Line, Module = module, Condition = x.Condition });
+                cleaned.AddRange(range);
+                _breakpoints = cleaned.ToArray();
+            }
         }
 
         public bool FindBreakpoint(string module, int line)
-            => _breakpoints.Find(x => x.Module.Equals(module) && x.LineNumber == line) != null;
+            => Find(module, line) != null;
 
+        // Точку могли снять между FindBreakpoint и GetCondition
         public string GetCondition(string module, int line)
-            => _breakpoints.Find(x => x.Module.Equals(module) && x.LineNumber == line).Condition;
+            => Find(module, line)?.Condition;
+
+        private BreakpointDescriptor Find(string module, int line)
+            => Array.Find(_breakpoints, x => x.Module.Equals(module) && x.LineNumber == line);
 
         public void Clear()
         {
-            _breakpoints.Clear();
-            _exceptionBreakpointsFilters.Clear();
+            lock (_lock)
+            {
+                _breakpoints = Array.Empty<BreakpointDescriptor>();
+            }
+            _exceptionBreakpointsFilters = new Dictionary<string, string>();
         }
 
         public bool StopOnAnyException(string message)
@@ -55,7 +68,7 @@ public bool StopOnUncaughtException(string message)
 
         private bool NeedStopOnException(string filterId, string message)
         {
-            if (_exceptionBreakpointsFilters?.TryGetValue(filterId, out var condition) == true)
+            if (_exceptionBreakpointsFilters.TryGetValue(filterId, out var condition))
             {
                 if (string.IsNullOrEmpty(condition))
                     return true;
@@ -66,4 +79,4 @@ private bool NeedStopOnException(string filterId, string message)
             return false;
         }
     }
-}
+}

src/ScriptEngine.HostedScript/TemplateStorage.cs

@@ -6,6 +6,7 @@ This Source Code Form is subject to the terms of the
 ----------------------------------------------------------*/
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using OneScript.Contexts;
 using OneScript.Contexts.Enums;
@@ -22,28 +23,31 @@ namespace ScriptEngine.HostedScript
     public class TemplateStorage : GlobalContextBase<TemplateStorage>, IDisposable
     {
         private readonly ITemplateFactory _factory;
-        private readonly Dictionary<string, ITemplate> _templates = new Dictionary<string,ITemplate>();
+        // Макеты регистрируют библиотеки, которые могут загружаться во время работы, пока их читают другие потоки
+        private readonly ConcurrentDictionary<string, ITemplate> _templates = new ConcurrentDictionary<string,ITemplate>();
 
         public TemplateStorage(ITemplateFactory factory)
         {
             _factory = factory;
         }
-        
+
         public void RegisterTemplate(string file, string name, TemplateKind kind)
         {
             if (_templates.ContainsKey(name))
                 throw RuntimeException.InvalidArgumentValue(name);
-            
+
             var template = _factory.CreateTemplate(file, kind);
-            _templates.Add(name, template);
+            if (!_templates.TryAdd(name, template))
+            {
+                template.Dispose();
+                throw RuntimeException.InvalidArgumentValue(name);
+            }
         }
-        
+
         public void RegisterTemplate(string name, ITemplate template)
         {
-            if (_templates.ContainsKey(name))
+            if (!_templates.TryAdd(name, template))
                 throw RuntimeException.InvalidArgumentValue(name);
-
-            _templates.Add(name, template);
         }
 
 

src/ScriptEngine/Machine/Contexts/AutoContext.cs

@@ -5,6 +5,7 @@
 at http://mozilla.org/MPL/2.0/.
 ----------------------------------------------------------*/
 
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
 using OneScript.Contexts;
@@ -19,7 +20,7 @@
     {
         private static readonly ContextPropertyMapper<TInstance> _properties = new ContextPropertyMapper<TInstance>();
         private static readonly ContextMethodsMapper<TInstance> _methods = new ContextMethodsMapper<TInstance>();
-        private static readonly HashSet<int> _warnedDeprecatedMethods = new HashSet<int>();
+        private static readonly ConcurrentDictionary<int, byte> _warnedDeprecatedMethods = new ConcurrentDictionary<int, byte>();
         private static readonly TypeDescriptor _objectType = typeof(TInstance).GetTypeFromClassMarkup();
 
         protected AutoContext() : base(_objectType)
@@ -118,12 +119,11 @@
             {
                 throw RuntimeException.DeprecatedMethodCall(methodInfo.Name);
             }
-            if (_warnedDeprecatedMethods.Contains(methodNumber))
+            if (!_warnedDeprecatedMethods.TryAdd(methodNumber, 0))
             {
                 return;
             }
             SystemLogger.Write($"ВНИМАНИЕ! Вызов устаревшего метода {methodInfo.Name}");
-            _warnedDeprecatedMethods.Add(methodNumber);
         }
 
         public override void CallAsProcedure(int methodNumber, IValue[] arguments, IBslProcess process)

src/ScriptEngine/Machine/Contexts/ContextMethodMapper.cs

@@ -21,7 +21,8 @@ namespace ScriptEngine.Machine.Contexts
 
     public class ContextMethodsMapper<TInstance>
     {
-        private List<InternalMethInfo> _methodPtrs;
+        // Публикуется последним: кто увидел заполненный список, увидит и словарь имен
+        private volatile List<InternalMethInfo> _methodPtrs;
         private IdentifiersTrie<int> _methodNumbers;
 
         private readonly object _locker = new object();

src/ScriptEngine/Machine/Contexts/ContextPropertyMapper.cs

@@ -17,7 +17,8 @@ namespace ScriptEngine.Machine.Contexts
 {
     public class ContextPropertyMapper<TInstance>
     {
-        private List<PropertyTarget<TInstance>> _properties;
+        // Публикуется последним: кто увидел заполненный список, увидит и словарь имен
+        private volatile List<PropertyTarget<TInstance>> _properties;
         private IdentifiersTrie<int> _propertyNumbers;
 
         private readonly object _locker = new object();

src/ScriptEngine/Machine/GlobalInstancesManager.cs

@@ -7,14 +7,17 @@ This Source Code Form is subject to the terms of the
 
 using System;
 using System.Collections;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 
 namespace ScriptEngine.Machine
 {
     public class GlobalInstancesManager : IGlobalsManager
     {
-        private readonly Dictionary<Type, object> _instances = new Dictionary<Type, object>();
+        // Экземпляры добавляются и во время работы (ПодключитьВнешнююКомпоненту, ЗагрузитьБиблиотеку),
+        // пока другие потоки их читают
+        private readonly ConcurrentDictionary<Type, object> _instances = new ConcurrentDictionary<Type, object>();
 
         public void Dispose()
         {
@@ -30,12 +33,13 @@ public void Dispose()
 
         public void RegisterInstance(object instance)
         {
-            _instances.Add(instance.GetType(), instance);
+            RegisterInstance(instance.GetType(), instance);
         }
 
         public void RegisterInstance(Type type, object instance)
         {
-            _instances.Add(type, instance);
+            if (!_instances.TryAdd(type, instance))
+                throw new ArgumentException($"An item with the same key has already been added. Key: {type}");
         }
 
         public object GetInstance(Type type)

src/Tests/OneScript.Core.Tests/TemplateStorageTests.cs

@@ -0,0 +1,72 @@
+/*----------------------------------------------------------
+This Source Code Form is subject to the terms of the
+Mozilla Public License, v.2.0. If a copy of the MPL
+was not distributed with this file, You can obtain one
+at http://mozilla.org/MPL/2.0/.
+----------------------------------------------------------*/
+
+using System;
+using System.Collections.Concurrent;
+using System.Linq;
+using System.Threading;
+using FluentAssertions;
+using Moq;
+using OneScript.StandardLibrary.Binary;
+using ScriptEngine.HostedScript;
+using Xunit;
+
+namespace OneScript.Core.Tests
+{
+    public class TemplateStorageTests
+    {
+        [Fact]
+        public void TemplatesRegisteredFromManyThreadsAreKept()
+        {
+            const int threadsCount = 8;
+            const int templatesPerThread = 500;
+            var storage = new TemplateStorage(Mock.Of<ITemplateFactory>());
+            var errors = new ConcurrentQueue<Exception>();
+
+            // Так регистрируют макеты библиотеки, загружаемые из разных заданий
+            var threads = Enumerable.Range(0, threadsCount).Select(number => new Thread(() =>
+            {
+                try
+                {
+                    for (var i = 0; i < templatesPerThread; i++)
+                    {
+                        storage.RegisterTemplate($"Макет{number}_{i}", new StubTemplate());
+                    }
+                }
+                catch (Exception e)
+                {
+                    errors.Enqueue(e);
+                }
+            })).ToArray();
+
+            foreach (var thread in threads)
+            {
+                thread.Start();
+            }
+            foreach (var thread in threads)
+            {
+                thread.Join();
+            }
+
+            errors.Should().BeEmpty();
+            storage.GetTemplates().Should().HaveCount(threadsCount * templatesPerThread);
+        }
+
+        private class StubTemplate : ITemplate
+        {
+            public string GetFilename() => "";
+
+            public BinaryDataContext GetBinaryData() => throw new NotSupportedException();
+
+            public TemplateKind Kind => TemplateKind.File;
+
+            public void Dispose()
+            {
+            }
+        }
+    }
+}

src/Tests/OneScript.DebugProtocol.Test/BreakpointManagerTests.cs

@@ -0,0 +1,101 @@
+/*----------------------------------------------------------
+This Source Code Form is subject to the terms of the
+Mozilla Public License, v.2.0. If a copy of the MPL
+was not distributed with this file, You can obtain one
+at http://mozilla.org/MPL/2.0/.
+----------------------------------------------------------*/
+
+using System;
+using System.Collections.Concurrent;
+using System.Linq;
+using System.Threading;
+using FluentAssertions;
+using OneScript.DebugServices;
+using Xunit;
+
+namespace OneScript.DebugProtocol.Test
+{
+    public class BreakpointManagerTests
+    {
+        [Fact]
+        public void ConditionOfRemovedBreakpointIsNull()
+        {
+            var manager = new DefaultBreakpointManager();
+            manager.SetBreakpoints("module.os", new[] { (10, "А = 1") });
+            manager.FindBreakpoint("module.os", 10).Should().BeTrue();
+
+            // Отладчик снял точку между проверкой строки и чтением условия
+            manager.Clear();
+
+            manager.GetCondition("module.os", 10).Should().BeNull();
+        }
+
+        [Fact]
+        public void BreakpointsCanBeCheckedWhileDebuggerChangesThem()
+        {
+            const string module = "module.os";
+            var manager = new DefaultBreakpointManager();
+            var errors = new ConcurrentQueue<Exception>();
+            var stop = false;
+            const int scriptThreadsCount = 4;
+            using var readersRunning = new CountdownEvent(scriptThreadsCount);
+
+            // Потоки скриптов проверяют точки на каждой строке
+            var scriptThreads = Enumerable.Range(0, scriptThreadsCount).Select(_ => new Thread(() =>
+            {
+                var signaled = false;
+                try
+                {
+                    while (!Volatile.Read(ref stop))
+                    {
+                        for (var line = 1; line <= 20; line++)
+                        {
+                            if (manager.FindBreakpoint(module, line))
+                                manager.GetCondition(module, line);
+                        }
+                        manager.StopOnAnyException("ошибка");
+
+                        if (!signaled)
+                        {
+                            signaled = true;
+                            readersRunning.Signal();
+                        }
+                    }
+                }
+                catch (Exception e)
+                {
+                    errors.Enqueue(e);
+                }
+                finally
+                {
+                    if (!signaled)
+                        readersRunning.Signal();
+                }
+            }) { IsBackground = true }).ToArray();
+
+            foreach (var thread in scriptThreads)
+            {
+                thread.Start();
+            }
+
+            // Поток отладчика меняет точки, когда все потоки скриптов уже проверяют их
+            var readersStarted = readersRunning.Wait(TimeSpan.FromSeconds(10));
+            var breakpoints = Enumerable.Range(1, 20).Select(line => (line, "")).ToArray();
+            for (var i = 0; i < 20000; i++)
+            {
+                manager.SetBreakpoints(module, breakpoints);
+                manager.SetExceptionBreakpoints(new[] { ("all", "ошибка") });
+                manager.Clear();
+            }
+
+            Volatile.Write(ref stop, true);
+            foreach (var thread in scriptThreads)
+            {
+                thread.Join();
+            }
+
+            readersStarted.Should().BeTrue();
+            errors.Should().BeEmpty();
+        }
+    }
+}