Skip to content

BUG: Fix default value crash - #179

Merged
codingl2k1 merged 3 commits into
mainfrom
bugfix/fix_default_value_crash
Aug 22, 2026
Merged

BUG: Fix default value crash#179
codingl2k1 merged 3 commits into
mainfrom
bugfix/fix_default_value_crash

Conversation

@codingl2k1

Copy link
Copy Markdown

No description provided.

Add CPU parameters postprocessing in Server initialization.
Added tests to verify default and explicit thread counts in Llama server.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request integrates CPU parameter post-processing during the initialization of the Server class in Cython, mirroring the CLI's behavior to resolve negative thread counts (such as -1 for auto) to the actual number of math cores. This prevents potential crashes when the C++ server creates its threadpools. Additionally, unit tests have been added to verify both default and explicit thread count behaviors. Feedback suggests declaring the common_params parameter as not None in __cinit__ to avoid potential null pointer dereferences or segmentation faults.

@@ -3111,6 +3111,15 @@ cdef class Server:
cdef shared_ptr[CServer] svr

def __cinit__(self, CommonParams common_params):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent potential null pointer dereferences or segmentation faults when None is passed as common_params, declare the parameter with not None. In Cython, accessing attributes (like common_params.p) on a None object can lead to undefined behavior or crashes if nonecheck is disabled.

    def __cinit__(self, CommonParams common_params not None):

@codingl2k1
codingl2k1 merged commit 9d5f858 into main Aug 22, 2026
7 checks passed
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.

1 participant