Skip to content

Conversation

@DmitryMalishev
Copy link

The is the fix for the issue, when integration test for "qvac-lib-infer-llamacpp-llm" on Windows hangs.

This small missprint "ro" instead of "rb" caused undefined behavior on Windows: on the second opening a model file it always got identical file ID as was used for the first open. But fclose() already called at first open, so that second descriptor had not-null but still invalid ID, so that it hanged on fseek() method to detect file size.

Sorry, string numbers are incorrect bellow, as I added many logs and they are counted in my local repo state.

Calling from Llm-Addon
  └─> llama_model_load_from_file()                    [llama.cpp/src/llama.cpp:256]
       └─> create_disk_fileloader()                    [llama.cpp/src/llama.cpp:250]
            └─> llama_model_loader::llama_model_loader() [llama.cpp/src/llama-model-loader.cpp:503]
                 └─> gguf_file_load::gguf_file_load()   [llama.cpp/src/llama-model-load.cpp:10]
                      │
                      ├─> [FIRST OPEN PATH]
                      │   gguf_init_from_file()          [llama.cpp/ggml/src/gguf.cpp:806]
                      │    └─> ggml_fopen(fname, "rb")   [llama.cpp/ggml/src/ggml.c:560]
                      │         └─> _wfopen()             [Windows CRT]
                      │              ├─> Returns: FILE* = 00000231E523F1D0
                      │              └─> fd = 3
                      │    
                      │    gguf_init_from_file_impl()    [llama.cpp/ggml/src/gguf.cpp:799]
                      │     └─> Reads metadata from FILE*
                      │    
                      │    fclose(FILE* 00000231E523F1D0) [llama.cpp/ggml/src/gguf.cpp:820]
                      │     └─> Returns FILE* to CRT pool
                      │    
                      │    Sleep(50)                      [llama.cpp/ggml/src/gguf.cpp:826]
                      │     └─> Waits 50ms
                      │
                      └─> [SECOND OPEN PATH - HANGS HERE]
                          llama_file_disk constructor     [llama.cpp/src/llama-model-load.cpp:31]
                           └─> std::make_unique<llama_file_disk>(fname, "rb")
                                └─> llama_file_disk::llama_file_disk() [llama.cpp/src/llama-mmap.cpp:250]
                                     └─> pimpl(std::make_unique<impl>(fname, mode))
                                          └─> impl::impl()              [llama.cpp/src/llama-mmap.cpp:180]
                                               └─> ggml_fopen(fname, "rb") [ggml.c:560]
                                                    └─> _wfopen()          [Windows CRT]
                                                         ├─> Returns: FILE* = 00000231E523F1D0 ⚠️ REUSED!
                                                         └─> fd = 3 ⚠️ SAME FD!
                                               
                                               ✅ fp = 00000231E523F1D0
                                               ✅ Check fp != NULL passes
                                               
                                               seek(0, SEEK_END)         [llama-mmap.cpp:191]
                                                └─> _fseeki64(fp, 0, SEEK_END)
                                                     └─> ⚠️ **HANGS HERE** ⚠️
                                                         Operating on reused/corrupted FILE*

Copy link

@jesusmb1995 jesusmb1995 left a comment

Choose a reason for hiding this comment

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

Thanks for finding this bug and fixing it.

Indeed, if you look at upstream Llama.cpp it uses "rb":

    files.emplace_back(new llama_file(fname.c_str(), "rb"));

@jesusmb1995
Copy link

This PR also fixes OpenBLAS CI 🙂 , See successful CI https://github.com/tetherto/qvac-ext-lib-llama.cpp/actions/runs/18656998019/job/53188454321?pr=40

Please help review/merge @jpgaribotti @olyasir

@olyasir olyasir merged commit c9115b9 into tetherto:temp-latest Oct 21, 2025
43 of 47 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.

4 participants