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

Ensure execution is scheduled after compilation #10883

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

hubertp
Copy link
Collaborator

@hubertp hubertp commented Aug 23, 2024

Pull Request Description

Scheduling of jobs is asynchronous and nothing prevents it from scheduling execution before compilation leading to stale nodes in GUI. The scenario is easily reproducible by adding a Thread.sleep before EnsureCompiledJob requests compilation locks.

This change ensures that execution is only scheduled after compilation is finished, when an edit request is being processed.

Important Notes

This fix was prompted by me getting random stale nodes after edits:
Screenshot from 2024-08-23 16-53-26
(it would never recover unless another edit was made)

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.

Scheduling of jobs is asynchronous and nothing prevents it from
scheduling execution before compilation leading to stale nodes in GUI.
The scenario is easily reproducible by adding a `Thread.sleep` before
`EnsureCompiledJob` requests compilation locks.

This change ensures that execution is only scheduled after compilation
is finished, when an edit request is being processed.
@hubertp hubertp added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 23, 2024
@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label Aug 23, 2024
@mergify mergify bot merged commit 3d23e22 into develop Aug 23, 2024
41 of 42 checks passed
@mergify mergify bot deleted the wip/hubert/stale-computations branch August 23, 2024 18:19
hubertp added a commit that referenced this pull request Aug 27, 2024
Test case for #10883 which revealed problems with visualizations in
issue #10897.
4e6 pushed a commit that referenced this pull request Sep 2, 2024
Test case for #10883 which revealed problems with visualizations in
issue #10897.
4e6 pushed a commit that referenced this pull request Sep 2, 2024
Test case for #10883 which revealed problems with visualizations in
issue #10897.
4e6 pushed a commit that referenced this pull request Sep 5, 2024
Test case for #10883 which revealed problems with visualizations in
issue #10897.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants