Skip to content

Conversation

@pmachapman
Copy link
Collaborator

@pmachapman pmachapman commented Dec 8, 2025

This PR fixes a System.InvalidOperationException that occurs because the ScrText Setting's internal dictionary to map filenames to book numbers to is being populated while it is being read. See the Paratext source code for ProjectSettings.GetBookNumberFromFilename() for details of the cache implementation.

It is very difficult to recreate this crash locally (I have had it once or twice over the last year), but this bug occurs occasionally on live with a stack trace as follows:

System.InvalidOperationException Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct. 
    unknown System.ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported()
    unknown System.Collections.Generic.Dictionary<TKey, TValue>.FindValue(TKey key)
    unknown Paratext.Data.ProjectSettingsAccess.ProjectSettings.GetBookNumberFromFilename(string bookFileName)
    unknown Paratext.Data.Repository.RevisionChangeInfo..ctor(VersionedText versionedText, HgRevision revision, List<T> revertedFiles)
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextService.cs:2277 SIL.XForge.Scripture.Services.ParatextService+<GetRevisionHistoryAsync>d__81.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    /home/runner/work/web-xforge/web-xforge/src/SIL.XForge.Scripture/Services/ParatextService.cs:2307 SIL.XForge.Scripture.Services.ParatextService+<GetRevisionHistoryAsync>d__81.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore<TResult>.ThrowForFailedGetResult()
    unknown SIL.XForge.Scripture.Services.ParatextService+<GetRevisionHistoryAsync>d__81.System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult(short token)
    unknown Microsoft.AspNetCore.Mvc.NewtonsoftJson.AsyncEnumerableReader+<ReadInternal>d__5<T>.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown Microsoft.AspNetCore.Mvc.NewtonsoftJson.AsyncEnumerableReader+<ReadInternal>d__5<T>.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown System.Runtime.CompilerServices.TaskAwaiter<TResult>.GetResult()
    unknown Microsoft.AspNetCore.Mvc.Formatters.NewtonsoftJsonOutputFormatter+<WriteResponseBodyAsync>d__13.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeResultAsync>g__Logged|22_0>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeNextResultFilterAsync>g__Awaited|30_0>d<TFilter, TFilterAsync>.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext<TFilter, TFilterAsync>(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeResultFilters>g__Awaited|28_0>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeNextResourceFilter>g__Awaited|25_0>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(ref State next, ref Scope scope, ref object state, ref bool isCompleted)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeFilterPipelineAsync>g__Awaited|20_0>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeAsync>g__Logged|17_1>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker+<<InvokeAsync>g__Logged|17_1>d.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Authorization.AuthorizationMiddleware+<Invoke>d__11.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Authentication.AuthenticationMiddleware+<Invoke>d__6.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Localization.RequestLocalizationMiddleware+<Invoke>d__5.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware+<Invoke>d__3.MoveNext()
    unknown System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    unknown System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task, ConfigureAwaitOptions options)
    unknown Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddlewareImpl+<<Invoke>g__Awaited|10_0>d.MoveNext()

See:


This change is Reviewable

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.77%. Comparing base (16b33f4) to head (0283a46).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3605   +/-   ##
=======================================
  Coverage   82.77%   82.77%           
=======================================
  Files         610      610           
  Lines       37234    37235    +1     
  Branches     6080     6104   +24     
=======================================
+ Hits        30819    30820    +1     
  Misses       5498     5498           
  Partials      917      917           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@RaymondLuong3 RaymondLuong3 self-assigned this Dec 9, 2025
Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RaymondLuong3 reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pmachapman)


src/SIL.XForge.Scripture/Services/ParatextService.cs line 2260 at r1 (raw file):

            // The ScrText Settings maintains a non-thread safe dictionary cache of book filenames to book numbers.
            // We initialize it here to prevent issues when iterating over revisions below.
            _ = scrText.Settings.GetBookNumberFromFilename(string.Empty);

Looking at the paratext code, it looks like using the empty string should be fine and it will initiate the caching of the book and filenames.

Code quote:

            _ = scrText.Settings.GetBookNumberFromFilename(string.Empty);

@RaymondLuong3 RaymondLuong3 added the do not merge See PR description and/or comments for explanation label Dec 9, 2025
@RaymondLuong3 RaymondLuong3 force-pushed the fix/history-thread-safety-crash branch from 29479ed to 775523a Compare December 9, 2025 19:24
@pmachapman pmachapman force-pushed the fix/history-thread-safety-crash branch from 775523a to 0283a46 Compare December 9, 2025 22:30
@pmachapman pmachapman merged commit c4a8e75 into master Dec 9, 2025
23 checks passed
@pmachapman pmachapman deleted the fix/history-thread-safety-crash branch December 9, 2025 22:45
@pmachapman pmachapman removed the do not merge See PR description and/or comments for explanation label Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants