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

#592 vtAbort if LB compile-time disabled but runtime enabled #593

Merged
merged 2 commits into from
Nov 26, 2019

Conversation

lifflander
Copy link
Collaborator

Fixes #592

// actually executes
theSched->enqueue([this]{
this->checkForArgumentErrors();
});
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this a bit racy? How do you ensure this is after whatever last point that arguments could safely be modified?

The principled solution to that issue would distinguish the name and type of "structure containing arguments as they are being processed" from "structure containing arguments as they should be used to initialize and execute", with the latter of course being immutable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@pbmille You want beta.4 merged soon, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this only works if EMPIRE doesn't call into the scheduler before modifying the args. We are looking for a solution where EMPIRE doesn't have to change.

Copy link
Collaborator

@nlslatt nlslatt left a comment

Choose a reason for hiding this comment

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

This solves my problem.

@lifflander lifflander merged commit d9791b0 into develop Nov 26, 2019
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.

Error out if LB is compile-time disabled and runtime-enabled
3 participants