Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llama : fix ctrl+c handler #11731

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

llama : fix ctrl+c handler #11731

wants to merge 1 commit into from

Conversation

magicse
Copy link
Contributor

@magicse magicse commented Feb 7, 2025

ref #11605, ggml-org/llama.vim#35

For correct processing ctrl+c signal

Make sure to read the contributing guidelines before submitting a PR

For correct processing ctrl+c signal
@ngxson
Copy link
Collaborator

ngxson commented Feb 7, 2025

Can you explain why this change is needed and which case does it fix?

@ngxson
Copy link
Collaborator

ngxson commented Feb 7, 2025

What you explained is not related to this change. I don't see why ctrl+c handler is related to that

@magicse
Copy link
Contributor Author

magicse commented Feb 7, 2025

Wow, sorry about that! I made two changes.

Regarding these changes:
We attempted to register the Ctrl+C event after ctx_server.queue_tasks.start_loop(). As a result, the user was no longer able to interrupt the process.

To allow the user to use Ctrl+C at any time, I think we need to register the event earlier in the process.

@ngxson
Copy link
Collaborator

ngxson commented Feb 7, 2025

Wow, sorry about that! I made two changes.

That's why you should have a proper title and description for each PR.

@ngxson
Copy link
Collaborator

ngxson commented Feb 7, 2025

This introduce another bug, now I can no longer interrupt model loading.

@ngxson ngxson changed the title Update server.cpp llama : fix ctrl+c handler Feb 7, 2025
@ggerganov
Copy link
Owner

ref #11605

@magicse
Copy link
Contributor Author

magicse commented Feb 7, 2025

I'll double check everything today. Could you please clarify what operating system you use?

@ngxson
Copy link
Collaborator

ngxson commented Feb 7, 2025

I'm using MacOS. It should be the same on all platforms. I think you moved the handler to before model loading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants