Skip to content

Fixing stuck _isInternalSet flag - #1394

Merged
beto-rodriguez merged 2 commits into
Live-Charts:devfrom
BHandle:FixingStuckInternalSet
Sep 16, 2024
Merged

Fixing stuck _isInternalSet flag#1394
beto-rodriguez merged 2 commits into
Live-Charts:devfrom
BHandle:FixingStuckInternalSet

Conversation

@BHandle

@BHandle BHandle commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

Fixes three cases where logic can flag that internal changes are being made to ChartElement objects and escape the scope without reverting the _isInternalSet flag. I was actually encountering this on a project where I set the stroke color of a LineSeries based on a condition in the code. Due to the timing involved, there was a race condition where I was updating the stroke color after the CartesianChart escaped due to the empty bounds with the flag still set, preventing my change from ever being applied.

@BHandle

BHandle commented Dec 8, 2023

Copy link
Copy Markdown
Contributor Author

Personally, I think _isInternalSet should be wrapped in a disposable class that can be set in a using() context to ensure it never escapes the scope, even via exception. Additionally, it would ensure that this sort of escape could never happen again. If you're interested in such a change, let me know and I can make the necessary changes.

Thanks!

@BHandle BHandle changed the title Fixing stuck internal set Fixing stuck _isInternalSet flag Dec 8, 2023
@beto-rodriguez
beto-rodriguez changed the base branch from master to dev September 16, 2024 00:32
@beto-rodriguez

Copy link
Copy Markdown
Collaborator

Late but merged. thanks for the PR!

I think _isInternalSet should be wrapped in a disposable class that can be set in a using() context

That sounds amazing to prevent this kind of issues, sure a PR is welcomed! (I promise to merge sooner 🤣)

@beto-rodriguez
beto-rodriguez merged commit ccad632 into Live-Charts:dev Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants