some bug fixes from fuzzer finds - #289
Open
stephen-dwq wants to merge 1 commit into
Open
stephen-dwq wants to merge 1 commit into
stephen-dwq wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Bugs found using
test/fuzzer.Buffered loop-back edges could requeue nodes.
ingest_inputputs an edge in one slot. It then callsregister_ingested_input, which reads both slots. An edge that lands inready_next_iterstill makes the registry readready_signals, which this ingest does not change. For a node that already ran, that slot is still full, so the node goes back onready_names. The scheduler can then run the node a second time in one iteration, on a partial input set.Registry counts completion instead of entities
Triggered by bug 1, which can complete a node twice.
The guard holds only after the registry is done. A duplicate completion before that point still raises the count. The registry then reports the iteration complete while one entity never ran. The loop resets early, the inputs of that entity are dropped, and the request will hang.
Finished loops can loop indefinitely with empty outputs
Previously hidden by bug 2, which would eventually end the loop because each loop execution would raise completion count.
A loop on its last iteration sets
is_done = Trueand callsinner_registry.clear(). That call returns the registry to its start, so the registry of the body says the body did not start. A later edge is then ingested as usual, the node goes onready_names, and the body starts again.complete_iterruns a second time and sends the declared outputs again. The first completion cleared_cached_outputs, so those outputs carry no tensor. The body runs without end.Backlog drain ignores deferred removal
_filter_cap_and_scheduledoes not ignorepending_removesandheldRIDs.5, 6. Emptied chunk can be considered work or indefinitely parked
If all RIDs are dropped (e.g. all requests failed), it is still considered work, and scheduled. We could work on things with nothing in them.
Abandoned pre-plans leak streams in
KVManagerLabels made for segments in pre-plans are not added to the list that
clear_preplanconsults; so their streams remain unclaimed until request end.Zero-span segments don't get streams in
KVManagerFresh zero-span segments (intended as readop) make it past
admitbut cannot actually beplanbecause no associated stream.