Потокобезопасные общие кэши маршаллинга перечислений и нативного компилятора

[sfaqer]

SimpleEnumsMarshaller._gettersCache был статическим LruCache без блокировки, а LruCache даже при чтении переставляет узлы списка. При параллельном чтении свойства-перечисления, например Задание.Состояние из нескольких фоновых заданий, падали The LinkedList node does not belong to current LinkedList, NRE и порча Dictionary. После этого чтение перечислений могло падать уже и в одном потоке, до перезапуска процесса. Так же устроены статические кэши ExpressionHelpers в нативном компиляторе.

Кэш перечислений теперь ConcurrentDictionary, ReflectedMembersCache берет блокировку. Сам LruCache оставил без блокировки: кэш MachineInstance у каждой машины свой, а lock в нем давал +3% на Вычислить. Новые тесты в tasks.os и ReflectedMembersCacheTest без исправления падают.

Замеры, Release, медиана

	develop	ветка
1 млн чтений Задание.Состояние	200,5 мс	193,5 мс	0,965
200 тыс. Вычислить("1 + 1")	95,0 мс	95,0 мс	1,000
пустой цикл 1 млн (контроль)	95,0 мс	93,5 мс	0,984

Отдельные процессы, 4 прогрева и 4 замера, 5 раундов с чередованием. Шум по контролю около 2%. Чтение перечисления быстрее и в прошлой серии (0,957).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of reflected-member lookups when accessed concurrently.
  • Enum getter caching now supports concurrent access and no longer uses a fixed-size eviction limit.
  • Parallel reads of background task state are covered by concurrency tests.
  • Clarified that LRU caches are not thread-safe and require external synchronization when shared across threads.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: aed86498-7674-4316-9b07-d148c666784b

📥 Commits

Reviewing files that changed from the base of the PR and between dabf321 and 4042c7c.

📒 Files selected for processing (6)

• src/OneScript.Core/Commons/LruCache.cs
• src/OneScript.Native/Compiler/ReflectedMembersCache.cs
• src/ScriptEngine/Machine/Contexts/SimpleEnumsMarshaller.cs
• src/ScriptEngine/Machine/LruCache.cs
• src/Tests/OneScript.Dynamic.Tests/ReflectedMembersCacheTest.cs
• tests/tasks.os

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes document that LRU caches are not thread-safe, synchronize reflected-member cache lookups, and replace the enum getter LRU cache with a ConcurrentDictionary. Tests exercise parallel reflected-member lookups and task-state reads.

Changes

Concurrent cache access

Layer / File(s)	Summary
Reflected-member cache access src/OneScript.Core/Commons/LruCache.cs, src/ScriptEngine/Machine/LruCache.cs, src/OneScript.Native/Compiler/ReflectedMembersCache.cs, src/Tests/OneScript.Dynamic.Tests/ReflectedMembersCacheTest.cs	The LRU cache documentation states that shared caches require external locking. ReflectedMembersCache<T> locks cache lookups and searches. A test performs parallel lookups for six Math methods.
Enum getter cache and concurrent reads src/ScriptEngine/Machine/Contexts/SimpleEnumsMarshaller.cs, tests/tasks.os	The enum getter cache changes from a 32-entry LRU cache to a ConcurrentDictionary keyed by enum type. A task test performs parallel reads of Состояние.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 4042c

No actionable risk remains from the supplied evidence; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 4042c

Synchronizing shared lookups reduces the risk of cache corruption. The trade-off is that enum getters are no longer evicted, so memory use can grow with the number of enum types a host loads and uses.

Retained concerns

• Low · reliability · inferred: The process-wide enum getter cache no longer evicts compiled delegates. Hosts that continually admit and use distinct CLR enum types can retain those delegates beyond the former 32-entry limit, reducing shared-process memory headroom. The evidence does not establish that a lower-privilege caller can produce such types.

Security review details

Security Blast Radius

• inferred — The retention change affects a static cache shared within a process, rather than an individual conversion. Its effective exposure depends on the host's ability to load and use distinct enum types; tenant and deployment isolation are not established.

Security Findings and Attack Paths

• inferred — Repeatedly loading and using distinct external enum types could grow retained getter memory. The demonstrated route loads executable assemblies, however; the evidence does not show a less-privileged, script-value-only way to create arbitrary CLR types or a verified security exploit.

Trust Boundaries and Controls

• observed — The cache field remains private and enum conversion retains its object/type check. Assembly-loading authorization is not established by the inspected source.

Resilience and Maintainability Implications

• observed — The reflected-member lock encloses cache lookup, reflection search, and the missing-member exception path, while that cache retains its configured capacity.

Hardening Proposals

• proposed — If hosts permit sustained extension loading, establish an enum-type cardinality or cache-lifetime policy and measure retained getters under that workload. This is conditional hardening, not an observed exploit.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Заголовок точно описывает основное изменение: исправление потокобезопасности общих кэшей маршаллинга перечислений и нативного компилятора. Он краткий и связан с изменениями в SimpleEnumsMarshaller и R…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[EvilBeaver]

А зачем у нас два LruCache? Кажется один из них это атавизм и надо испоьзовать везде тот, который из Commons

[sfaqer]

Да, копия в ScriptEngine.Machine появилась позже, для кэша Выполнить/Вычислить, и использовалась только в MachineInstance. Убрал её в #1756.