rpc: remove backend handle from global map when freed

[chraac]

Found an app crash occurring when trying to reload a new llm model after another model has been unloaded.

When built with ASan, it shows a heap-buffer-overflow error in ggml_backend_rpc_get_device_memory. The relevant log is as follows:

READ of size 8 at 0x60e00039c6f8 thread T78
    #0 0x7666861ff0b3 in ggml_backend_rpc_get_device_memory (libllama.so+0x1ff0b3)
    #1 0x76668633644e  (libllama.so+0x33644e)
    #2 0x7666863af017 in llama_load_model_from_file (libllama.so+0x3af017)
    ...

Upon investigating the source code, discovered that in ggml-rpc.cpp, the ggml_backend_t instances are stored in a map called instances. However, inside the ggml_backend_rpc_free function, we forgot to remove the ggml_backend_t item from the map when it is freed:

ggml-rpc.cpp#L531

GGML_CALL static void ggml_backend_rpc_free(ggml_backend_t backend) {
    ggml_backend_rpc_context * rpc_ctx = (ggml_backend_rpc_context *)backend->context;
    ggml_backend_rpc_buffer_type_context * buft_ctx = (ggml_backend_rpc_buffer_type_context *)rpc_ctx->buft->context;
    delete buft_ctx;
    delete rpc_ctx->buft;
    delete rpc_ctx;
    delete backend;   // delete the backend object without removing it from map.
}

Here we insert ggml_backend_t into map:
ggml-rpc.cpp#L679

GGML_CALL ggml_backend_t ggml_backend_rpc_init(const char * endpoint) {
    ...

    instances[endpoint] = new ggml_backend {
        /* .guid      = */ ggml_backend_rpc_guid(),
        /* .interface = */ ggml_backend_rpc_interface,
        /* .context   = */ ctx
    };

    return instances[endpoint];
}

[chraac]

Should we lock the map when changing its content? Will it be accessed from multi thread?

[rgerganov]

Unfortunately this doesn't solve the problem in all cases. For example in llama-bench the backend is still used after calling ggml_backend_rpc_free. The proper behavior is to deallocate the backend when all buffers are freed and ggml_backend_rpc_free is called (see slaren's comment in PR #7435). However, it is not guaranteed that ggml_backend_rpc_free will be called last.

I am working on a fix and will submit a PR shortly.

[chraac]

Unfortunately this doesn't solve the problem in all cases. For example in llama-bench the backend is still used after calling ggml_backend_rpc_free. The proper behavior is to deallocate the backend when all buffers are freed and ggml_backend_rpc_free is called (see slaren's comment in PR #7435). However, it is not guaranteed that ggml_backend_rpc_free will be called last.

I am working on a fix and will submit a PR shortly.

so there's another place that keep the ggml_backend_t after it was freed? for such case, it would be better to use the shared_ptr and weak_ptr.

[rgerganov]

please take a look at PR #7562