Skip to content

server : add pidfile option #14242

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions common/arg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3373,5 +3373,9 @@ common_params_context common_params_parser_init(common_params & params, llama_ex
}
).set_examples({LLAMA_EXAMPLE_SERVER}));

add_opt(common_arg({ "--pidfile" }, "FILE", "path to PID file for server process",
[](common_params & params, const std::string & value) { params.pidfile = value; })
.set_examples({ LLAMA_EXAMPLE_SERVER }));

return ctx_arg;
}
1 change: 1 addition & 0 deletions common/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ struct common_params {
std::string hostname = "127.0.0.1";
std::string public_path = ""; // NOLINT
std::string chat_template = ""; // NOLINT
std::string pidfile = ""; // path to PID file for server process // NOLINT
bool use_jinja = false; // NOLINT
bool enable_chat_template = true;
common_reasoning_format reasoning_format = COMMON_REASONING_FORMAT_DEEPSEEK;
Expand Down
97 changes: 92 additions & 5 deletions tools/server/server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,32 @@
// mime type for sending response
#define MIMETYPE_JSON "application/json; charset=utf-8"

// auto generated files (see README.md for details)
#include "index.html.gz.hpp"
#include "loading.html.hpp"
#include <signal.h>

#include <atomic>
#include <chrono>
#include <cinttypes>
#include <condition_variable>
#include <cstddef>
#include <cinttypes>
#include <deque>
#include <memory>
#include <mutex>
#include <signal.h>
#include <thread>
#include <unordered_map>
#include <unordered_set>

// auto generated files (see README.md for details)
#include "index.html.gz.hpp"
#include "loading.html.hpp"

#ifdef _WIN32
#include <process.h>
#define getpid _getpid
#define pid_t int;
#else
#include <unistd.h>
#endif

using json = nlohmann::ordered_json;

constexpr int HTTP_POLLING_SECONDS = 1;
Expand Down Expand Up @@ -3691,6 +3700,77 @@ inline void signal_handler(int signal) {
shutdown_handler(signal);
}

static bool check_pid_alive(const pid_t pid) {
if (pid <= 0) {
return false;
}

// Process is alive or exists but is inaccessible
if (kill(pid, 0) == 0 || errno == EPERM) {
return true; // Process is alive
}

return false; // Process does not exist or other error
}

class PidFile {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe move it to utils.hpp to avoid pollute server.cpp with non-server code

public:
FILE * file = nullptr;
std::string fname;
bool rm = false;

FILE * open(const std::string & filename, const char * mode, const bool r = false) {
file = ggml_fopen(filename.c_str(), mode);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already had many functions to handle file i/o in common.cpp, should reuse it instead of inventing a new one

fname = filename;
rm = r;

return file;
}

void close() {
fclose(file);
file = nullptr;

if (rm) {
// Remove stale pidfile
unlink(fname.c_str());
}
}

~PidFile() {
if (file) {
close();
}
}
};

static bool is_old_pid_alive(const std::string & filename) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can move all static function into the class PidFile

pid_t oldpid = 0;
PidFile f;
if (f.open(filename, "r")) {
if (fscanf(f.file, "%d", &oldpid) == 1) {
if (check_pid_alive(oldpid)) {
LOG_ERR("Process already running with PID %d\n", oldpid);
return true;
}
}
}

return false;
}

static int create_pidfile(const std::string & pidfile, PidFile & f) {
if (!f.open(pidfile.c_str(), "w", true)) {
LOG_ERR("Unable to open pidfile %s: %s\n", pidfile.c_str(), strerror(errno));
return -1;
}

fprintf(f.file, "%d\n", getpid());
fflush(f.file);

return 0;
}

int main(int argc, char ** argv) {
// own arguments required by this example
common_params params;
Expand All @@ -3699,6 +3779,13 @@ int main(int argc, char ** argv) {
return 1;
}

PidFile f;
if (!params.pidfile.empty()) {
Comment on lines +3782 to +3783
Copy link
Collaborator

Choose a reason for hiding this comment

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

the variable is not used outside its scope, so it's better to move inside the if {..}

Suggested change
PidFile f;
if (!params.pidfile.empty()) {
if (!params.pidfile.empty()) {
PidFile f;

Copy link
Collaborator Author

@ericcurtin ericcurtin Jun 17, 2025

Choose a reason for hiding this comment

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

No there's a reason it's in the outer scope, we want the destructor to attempt to delete the file when main() exits, not any sooner

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm ok maybe put this as a comment for visibility

if (is_old_pid_alive(params.pidfile) || create_pidfile(params.pidfile, f)) {
return 1;
}
}

common_init();

// struct that contains llama context and inference
Expand Down
Loading