Skip to content

server : fix server_tokens::push_back infinite loop and placeholder copy - #29956

Open
sanjeevafk wants to merge 1 commit into
ggml-org:masterfrom
sanjeevafk:fix-server-tokens-media-chunks
Open

sanjeevafk wants to merge 1 commit into
ggml-org:masterfrom
sanjeevafk:fix-server-tokens-media-chunks

Conversation

@sanjeevafk

@sanjeevafk sanjeevafk commented Oct 4, 2026 •

Copy link
Copy Markdown

Overview

Fixes #29865 in server_tokens::push_back(const server_tokens & tokens):

  • Insert tokens in bulk using this->tokens.insert(...) rather than calling the single-token overload push_back(tokens[i]), which throws an error on LLAMA_TOKEN_NULL placeholders.
  • Add the missing ++it in the media map loop to fix an infinite loop when copying media chunks.
  • Make the argument a const reference and use it->second.get().

Note: Dropped the keep_first change from this PR to avoid overlap with #24076.

Additional information

Tested locally appending server tokens with media chunks; confirmed the loop terminates properly and placeholders copy without throwing.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - Used an AI assistant to help investigate the hang and trace token copy logic. Manually tested and reviewed.

@sanjeevafk
sanjeevafk requested a review from a team as a code owner October 4, 2026 16:32
@github-actions github-actions Bot added the server label Oct 4, 2026
@ggml-gh-bot

ggml-gh-bot Bot commented Oct 4, 2026

Copy link
Copy Markdown

Hi @sanjeevafk, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.

  • AI-generated content: While code is allowed to be generated by AI, please write the PR description and commit messages on your own without the help of AI.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@ggml-gh-bot ggml-gh-bot Bot added the draft PR will be changed to draft by github-actions bot label Oct 4, 2026
@github-actions
github-actions Bot marked this pull request as draft October 4, 2026 16:37
@github-actions github-actions Bot removed the draft PR will be changed to draft by github-actions bot label Oct 4, 2026
@sanjeevafk
sanjeevafk force-pushed the fix-server-tokens-media-chunks branch from a1ef2c6 to 1763b07 Compare October 4, 2026 16:43
@sanjeevafk

Copy link
Copy Markdown
Author

Updated the PR description and commit message to match the template and project guidelines.

@sanjeevafk
sanjeevafk marked this pull request as ready for review October 4, 2026 16:46
@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Hi, the keep_first fix is already covered by #24076, open since June

…nsertion

- Copy tokens in bulk via vector::insert instead of calling single-token push_back, which threw an error on LLAMA_TOKEN_NULL media placeholders
- Increment the media map iterator (++it) to fix the infinite loop
- Accept const server_tokens & and use it->second.get() directly

Fixes ggml-org#29865
@sanjeevafk
sanjeevafk force-pushed the fix-server-tokens-media-chunks branch from 1763b07 to ea0eae2 Compare October 4, 2026 17:42
@sanjeevafk sanjeevafk changed the title server : fix server_tokens media chunk handling in push_back and keep_first server : fix server_tokens::push_back infinite loop and placeholder copy Oct 4, 2026
@sanjeevafk

Copy link
Copy Markdown
Author

@ServeurpersoCom Thanks for catching that! I didn't realize #24076 had already addressed the keep_first boundary.

I've reverted the keep_first change here to avoid any overlap, and updated this PR to focus strictly on fixing the push_back infinite loop and LLAMA_TOKEN_NULL placeholder copy for #29865.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Note that on master this path is only reached by format_prompt_rerank with text-only tokens, so this is a latent fix with no reachable repro today.

@sanjeevafk

Copy link
Copy Markdown
Author

Ok then, I will leave it up to the maintainers whether to merge it proactively or wait till the multimodal rerank expands. Thanks for the catch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misc. bug: infinite loop in server_tokens::push_back(server_tokens&) when the appended tokens contain media

2 participants