Add test for "Infinite recursion" in Job prim ordering#1708
Open
Sam May (ag-eitilt) wants to merge 1 commit intomasterfrom
Open
Add test for "Infinite recursion" in Job prim ordering#1708Sam May (ag-eitilt) wants to merge 1 commit intomasterfrom
Sam May (ag-eitilt) wants to merge 1 commit intomasterfrom
Conversation
Collaborator
Author
|
Note to self: This test should live alongside the similar ones in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reproducer for #1670. Specifically, I haven't been able to get a MVP which shows the full "Infinite recursion..." error, but by comparing
testPassLaunchandtestAbruptLaunch, we can see the conditions for that recursion in the latter returning an unresolved<future>without any wrappingPassorFail1 -- my guess is that it's a future which escapes the scheduler by being returned unresolved from another future, and so on infinitely, and specific cases are able to unwrap enough of them to trigger the loop detection.I don't think this is something we want to merge until it's fixed, but this test case is going to be helpful if we ever try to address this, so we either leave this PR somewhere we'll be able to find it later, or maybe we do merge it in the mean time after switching to a
fail.shscript, and document that we eventually want the test to pass instead but that it's a known backburnered failure until then.This whole saga is connected to #160, and as we found last year that detection does definitely work, so I don't know why it's able to escape through this logic; it's not the Wake version since I was running into the same result on the 44.0.4 tag where the previous known problem was happening. Also, I've strangely suddenly started hitting the job.cpp:1236 assert during the should-fail test just before pushing the branch, when everything I did up until the final preparations had it returning the expected
Pass "{msg}".Footnotes
The lack of a wrapper is a visible sign that the execution of the test case never completed; with a bit more indirection I can get the output to print
Pass <future>, for example, where it's clearly reaching the end of execution but just not finalizing the block's return value. ↩