fix(cpp): reject input a scan loop cannot make progress on - #418
Closed
j-sperling wants to merge 1 commit into
Closed
fix(cpp): reject input a scan loop cannot make progress on#418j-sperling wants to merge 1 commit into
j-sperling wants to merge 1 commit into
Conversation
Each of the three scan loops dispatches on is.peek() and, in its default
branch, extracts with operator>>. A failed extraction consumes nothing, so
peek() returns the same byte on the next pass. The loops terminate only on
']', which malformed input never supplies, so truncated input spins forever
and allocates on every iteration.
Reproducer, against an unpatched header:
std::stringstream in("[1,2,3"); // no closing bracket
std::vector<int> v;
LeetCodeIO::scan(in, v); // never returns
scan_list and scan_tree have the same structure and the same behaviour.
Checking for progress at the top of each loop turns the hang into an
immediate diagnostic on stderr. Valid input never reaches the check in a
failed state, because a well-formed array always has a ']' left to consume,
and the trailing ignore() calls in generated drivers run after the loop has
returned.
This also fixes a misleading report from `leetgo test`: malformed input
currently surfaces as a three-second time limit, sending users after a
performance problem that does not exist.
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.
Problem
The three scan loops in
testutils/cpp/LC_IO.hhang forever on malformed or truncated input.Each loop dispatches on
is.peek()and, in itsdefault:branch, extracts withoperator>>. A failed extraction consumes nothing, sopeek()returns the same byte on the next pass. The loops terminate only on']', which malformed input never supplies — so the loop spins, allocating on every iteration.Standalone reproducer, no leetgo workspace needed:
Helper::scan_listandHelper::scan_treehave the identical structure and the identical behaviour.[1,null,3]scanned asvector<int>hangs the same way, sincenfails extraction in the array loop.Fix
require_progress()checks at the top of each loop and exits with a diagnostic instead of spinning.Three loops is complete coverage rather than partial: the scalar
scanoverloads either delegate to these loops or use>>/std::quoted, whose failures set failbit and surface at the enclosing loop's next check.The check cannot fire on valid input — a well-formed array always has a
']'left to consume, and the trailingcin.ignore()calls in generated drivers run after the loop has already returned.Why it is worth fixing
Beyond the hang itself, this is currently mis-reported.
leetgo testshows malformed input as a three-second time limit exceeded, which sends users looking for a performance problem that does not exist. It also makes a gen/brute stress loop wedge silently, since a generator bug produces exactly this input.Notes
<cstdlib>and<iostream>explicitly rather than relying on<bits/stdc++.h>being pulled in first by generated code.g++ -std=c++17 -O2 -o tests tests.cpp && ./tests): all 15 existing tests pass.tests/tests.cppis single-process with no death-test facility. Happy to add afork()-based test, or to switch the failure to a throw so it can be caught in-process, if you would prefer either.