Skip to content

Tokenizer fallback and robustness fixes - #151

Open
CodeAKrome wants to merge 161 commits into
NotPunchnox:Betafrom
CodeAKrome:tokenizer-fallback-robustness
Open

Tokenizer fallback and robustness fixes#151
CodeAKrome wants to merge 161 commits into
NotPunchnox:Betafrom
CodeAKrome:tokenizer-fallback-robustness

Conversation

@CodeAKrome

Copy link
Copy Markdown

Summary

This PR adds robust tokenizer handling and a null-safety fix across two API files:

  • src/rkllama/api/rkllm.py: Null-check for processor config value before calling .lower(), preventing AttributeError when the value is None.
  • src/rkllama/api/server_utils.py: Multi-layer tokenizer resilience in get_tokenizer() — tries HuggingFace with HF_TOKEN support, falls back to google/gemma-3-4b-it, and finally creates a FallbackTokenizer if all else fails. Local tokenizer loading is similarly wrapped. All tokenizer access is now validated with clear error messages.

Changes

rkllm.py

  • Added processor and guard before processor.lower() check

server_utils.py

get_tokenizer() — Robust loading:

  • Supports HF_TOKEN env var for gated HuggingFace repos
  • Downloads from HuggingFace with try/except; falls back to google/gemma-3-4b-it tokenizer source on failure
  • Creates FallbackTokenizer if both sources fail (prevents hard crashes)
  • Wraps local tokenizer loading with FallbackTokenizer fallback
  • Wraps save_pretrained() with try/except to avoid write failures
  • Validates tokenizer is not None and has chat_template, raising a clear ValueError otherwise

_create_fallback_tokenizer() — New static method:

  • Implements a basic PreTrainedTokenizer with 32k vocab, bos/eos/pad/unk tokens, and a chat template
  • Allows inference to proceed when no proper tokenizer is available

Quote normalization:

  • Consistent single-quote style across all rkllama.config.get() calls in ChatEndpointHandler, GenerateEndpointHandler, and EmbedEndpointHandler

NotPunchnox and others added 30 commits April 12, 2025 17:01
Support tag names as model aliases in Ollama API
- Dockerfile cleanup and copy extra configuration files.
- Make sure to use same USE_CONDA configuration value in all files.
  - When USE_CONDA=fase is set, this avoids downloading Conda
    on the first start as setup.sh propagates the variable value
    to the rkllama script.
- Docker Compose:
  - Use repository container image.
  - Expose also Ollama port.
  - Use standard file name.
- Ensure setup.sh stops on errors, unset variables,
  or failed pipeline commands more robust execution.
- Update README.md.
feat: Add HuggingFace to RKLLM converter and improve Docker support
fix: correct indentation in pull_model function
fix: use sudo for sed command to modify rkllama executable
Tool Calling support. Fix context length for  RKLLM 1.1.4.
Now support Tool Call with Streaming. Option to disable thinking in models  that suport it.
danielferr85 and others added 30 commits March 25, 2026 10:10
…rformance. Fix translations not using pipe. Relocation of <image> tag in template for multimodal
Vision encoder running in the same rkllm worker process for better pe…
Fix build script so that docker images are published
…32-inputs

Fix float64 numpy inputs before RKNN image inference
Add step to normalize Docker image name to lowercase for consistency in image handling.
…rocesses

stop_worker() called process.join() with no timeout, so a stuck worker
(blocked in RKLLM inference or pipe read) would hang forever, leaking
NPU memory and permanently blocking the TTL monitor thread.

- Add 30s timeout to process.join(), force-kill with SIGKILL if needed
- Wrap pipe sends in try/except for already-crashed workers
- Close pipe after process exit to avoid fd leaks
- Add exception handling in unload_expired_models() so one stuck worker
  can't block cleanup of others
Add loaded_by to WorkerModelInfo, auto-populated from X-Loaded-By
header or User-Agent on each request. Shown in /api/ps response
so operators can see exactly what process loaded each model.
add_worker() had a race condition: concurrent requests for the same
model would all pass the 'not in self.workers' check before any of
them added to the dict, spawning multiple workers. Only the last one
got tracked — the rest became orphaned processes leaking NPU memory.

Wrap the check-and-create in a threading.Lock so only one worker is
ever spawned per model. This was the root cause of the repeated
orphaned worker processes.
Worker processes spawned by multiprocessing.Process become orphans
(reparented to init) if the rkllama parent dies from SIGKILL, crash,
or power loss. These orphans keep holding NPU memory forever and
can't be cleaned up without manual intervention.

Three-layer defence:

1. PR_SET_PDEATHSIG on each worker — Linux kernel sends SIGTERM the
   instant the parent dies, no matter how. This is the strongest
   guarantee; tested with kill -9 and the worker dies immediately.

2. Signal handlers + atexit on parent — clean shutdown on SIGTERM,
   SIGINT, or normal exit stops all workers gracefully before the
   parent dies.

3. Orphan scan on startup — find any rkllama_server processes whose
   PPID is 1 (orphaned from a previous run) and kill them. Protects
   against the rare case where PDEATHSIG didn't fire (e.g. running
   on non-Linux, or kernel quirk).

Verified with kill -9 on parent: worker process is gone in <1 second.
…ectedly

A worker process can die while the parent is alive (RKLLM C++ segfault,
kernel OOM, external SIGKILL). The parent's self.workers dict kept the
stale entry indefinitely, so api/ps reported the model as 'loaded' even
though no process was backing it. Loading the same model again would be
blocked by the 'already loaded' check.

Add a reaper pass to the monitor loop that calls process.is_alive() on
each tracked worker. If a worker is dead (exitcode set, not alive), log
the exit, join the zombie, close the pipe, and drop the entry from the
dict so the system reflects reality.
Fix worker lifecycle bugs causing NPU memory leaks
…process)

PR_SET_PDEATHSIG is bound to the *thread* that forked the child, not to the
parent process (man 2 prctl: "the 'parent' in this case is considered to be
the thread that created this process").

rkllama_server runs Flask with threaded=True, so Process.start() for a worker
is executed from a short-lived request-handler thread. As soon as the request
finishes and its thread exits, the kernel delivers SIGTERM to the worker, the
inherited shutdown handler cascades into stop_all() / sys.exit(0), and the
worker dies after serving a single request. The next /api/embed hits the
dying worker, waits the 30s stop_worker timeout, and returns 500.

Turn _set_parent_death_signal() into a documented no-op. Orphan-worker
protection continues to work via _kill_orphaned_workers() at startup.

Fixes NotPunchnox#117.

Co-Authored-By: Claude <noreply@anthropic.com>
…hsig-flask-threaded

fix: disable PR_SET_PDEATHSIG (kernel binds it to parent thread under Flask threaded=True)
python:3.12-slim (Debian Trixie) ARM64 packages are compiled with Pointer
Authentication (PAC, ARMv8.3-A).  RK3588 (Cortex-A76/A55) is ARMv8.2-A
and does not implement PAC.  The Linux kernel returns ENOEXEC for
PAC-guarded ELF binaries on hardware without PAC support, causing an
'exec format error' even though the image is correctly built for arm64.

Ubuntu 24.04 ARM64 packages do not use PAC and run correctly on RK3588.

Use a venv to work around Ubuntu 24.04 PEP 668 pip restrictions.
…che contamination

On an ubuntu-24.04-arm native runner, setting platforms: linux/arm64 in
setup-buildx-action forces a docker-container driver (QEMU-backed) instead
of using the host builder.  This causes the layer cache to be shared with
amd64 builds, pulling stale amd64 base OS layers into an arm64 image.

Rename the registry cache key from :buildcache to :buildcache-arm64 to
avoid reusing the stale amd64 cache that accumulated before this fix.
…ldx comment

Revert action major versions to what was present before this PR:
- actions/checkout@v6
- sigstore/cosign-installer@v4.1.1
- docker/setup-buildx-action@v4
- docker/login-action@v4
- docker/metadata-action@v6

These downgrades were not required for the ARM64/PAC fix and could
introduce unintended behaviour differences (Copilot review feedback).

Also fix the misleading comment on setup-buildx-action: the action uses
a docker-container builder by default, not a "host builder".  The
comment now correctly describes why omitting `platforms:` avoids
QEMU-backed cross-compilation and cache contamination.
The previous commit accidentally dropped the original workflow comments
(copyright header, step descriptions, URLs, sigstore notes) and
downgraded build-push-action from v7 to v6.

This commit restores all original comments verbatim and integrates the
ARM64 explanation as NOTE blocks within the existing comment sections,
keeping the diff minimal and reviewable.
fix: ubuntu:24.04 base image + remove platforms: flag to fix ARM64 on RK3588
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.