Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared DbContext causing concurrency-related exception #193

Closed
Tracked by #182
hoeyi opened this issue May 31, 2023 · 1 comment · Fixed by #198 or #200
Closed
Tracked by #182

Shared DbContext causing concurrency-related exception #193

hoeyi opened this issue May 31, 2023 · 1 comment · Fixed by #198 or #200
Assignees
Labels
area: data services Data worker services bug Something isn't working

Comments

@hoeyi
Copy link
Owner

hoeyi commented May 31, 2023

Describe the bug

Non-thread-safe use of FinanceDbContext can create scenarios where multiple select statements are called concurrently, throwing an InvalidOperationExcpetion.

To Reproduce
Steps to reproduce the behavior:

  1. Using the Pagination.razor component, quickly page through results.
  2. After a couple of hits, encounter the 'loading' control.
  3. Check the console and see the exception raised.

Expected behavior

Note the docs indicate that DbContext is not thread-safe, and should be disposed after each instance. This needs to be better handled by the service layer so that disconnected entities may be used.

Bug-reproducing version can be found at commit ccc8493.

@hoeyi hoeyi added bug Something isn't working area: data services Data worker services labels May 31, 2023
@hoeyi
Copy link
Owner Author

hoeyi commented May 31, 2023

There may not be an ideal solution this. The problem is doing this while limiting unnecessary trips to the database. The only way that seems feasible is to track changes manually. This is how this was handled in a similar scenario here. Using a stack-like structure would also assist with managing 'undo/redo' requests for changes. Might need to keep that to only reversing deletes for the sake of time/priorities.

Issue dotnet/efcore#5536 discusses creation of something for out of the box, so keep it mind.

@hoeyi hoeyi self-assigned this Jun 3, 2023
hoeyi added a commit that referenced this issue Jun 4, 2023
…tion<T>

Add CollectionCommand and CollectionCommandHistory tracking for working forwards
and backwards through 'undo' and 'redo' operations. Supports changes to entity model
services responsible for adding/removing/updating multiple entities at once.

Related #193
hoeyi added a commit that referenced this issue Jun 8, 2023
…rvices

Remove the persisted DbContext classes in the entity model service layer and replace with
correct short-lived used of DbContext-derived classes. Introduces ChangeTracking for a
disconnected collection of  entities to be worked in batches of not more than a single page size.
Update tracking is not available.

Related #193
hoeyi added a commit that referenced this issue Jun 10, 2023
Changes pages that work with collections of entities to inherit from ModelListPage and
use the updated controller interface ICollectionController. Completes work necessary to
support pagination without crashing due to multi-threaded access to DbContext classes.

Resolves #193
@hoeyi hoeyi mentioned this issue Jun 10, 2023
hoeyi added a commit that referenced this issue Jun 10, 2023
* refactor(DAL): remove obsolete services

Remove entity model services where the models will be maintained via
a parent object service.

* test(DAL): remove tests for obsolete entity model services

* refactor(Controller): remove obsolete entity model controllers

* refactor(Blazor): remove unnecessary lock object

* feat(DAL): add class for tracking add/remove operations on an ICollection<T>

Add CollectionCommand and CollectionCommandHistory tracking for working forwards
and backwards through 'undo' and 'redo' operations. Supports changes to entity model
services responsible for adding/removing/updating multiple entities at once.

Related #193

* test(DAL): add tests for CollectionCommandHistory class

* refactor(DAL): remove obsolete CountryBatchWriterService

* refactor(DAL): refactor ICollectionCommandHistory and CommandHistoryEntry

Rename ICollectionCommandHistory to ICommandHistory.
Move CommandHistoryEntry to separate file.

* feat(DAL): add Clear method to ICommandHistory interface

* test(DAL): add test for ICommandHistory.Clear method

* feat(DAL): make ICommandHistory generic

Convert ICommandHistory to generic interface and update implementation.
Rename CollectionCommandHistory to CommandHistory.

* feat(DAL): add IChangeTracker implementation ot CommandHistory

Update CommandHistory to implement IChangeTracker. Class features are not exactly
aligned, but this approach was most expedient. Opportunity to refactor later.

* test(DAL): fix test name add tests for IChangeTracker methods

Rename CollectionCommandHistoryTest to CommandHistoryTest to mirror tested class.
Add tests for IChangeTracker methods implemented in CommandHistory<T>.

* feat(DAL): consolidate methods in IChangeTracker and ModelCollectionService

Add ModelCollectionService to replace ModelWriterBatchService and ModeBatchService
class. ModelCollectionServices implements add/remove change tracking for a disconnected entity set.

* fix(DAL): add missing resource file

* test(DAL): update IChangeTracker tests to use new GetChanges method

* feat(EntityModel): change DatabaseKey to a public extension method

* fix(DAL) remove shared and persisted DbContext classes from entity services

Remove the persisted DbContext classes in the entity model service layer and replace with
correct short-lived used of DbContext-derived classes. Introduces ChangeTracking for a
disconnected collection of  entities to be worked in batches of not more than a single page size.
Update tracking is not available.

Related #193

* test(DAL): fix tests related to removed IModelBatchService interface

* fix(Controllers): fix controller referneces to IModelBatchController

* refactor(DAL): move IModelCollectionService to separate file

* fix(DAL): resolve errors in service registration

* fix(Blazor): update collection pages to inherit ModelListPage

Changes pages that work with collections of entities to inherit from ModelListPage and
use the updated controller interface ICollectionController. Completes work necessary to
support pagination without crashing due to multi-threaded access to DbContext classes.

Resolves #193

* refactor(Blazor): move most page initialization methods to base class

Move majority of page initialization logic to base classes ModelListPage, ModelIndex,
and ModelPagedIndex. Prepares the UI layer for application of Paginator control.

* fix(DAL): fix missing constructor arguments for SecurityExchangeBatchService

* refactor(DAL): standardize names for ModelCollectionService classes

* feat(Blazor): add paginator control to base index pages

* design(Blazor): fix form-control.button rule to preserve size of button icons

* feat(Blazor): add paginator control to remaining collection pages
@hoeyi hoeyi linked a pull request Jun 10, 2023 that will close this issue
@hoeyi hoeyi mentioned this issue Jun 11, 2023
@hoeyi hoeyi linked a pull request Jun 11, 2023 that will close this issue
@hoeyi hoeyi closed this as completed in d5c0923 Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: data services Data worker services bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant