retropilot
/
openpilot
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Params: new class FileLock (#20636) 

* new class LockFile

* lock->try_lock

* rebase master

* close if fd_ >=0

* rename try_lock to lock

* keep tryring if flock() return EINTR

* throw exception on error

* keep trying if open() gets interrupted by a signal

* close fd_ before throw flock exception

* add macro HANDLE_EINTR

* HANDLE_EINTR for open & write

* add errno in exception

* add destructor

* don't throw exception,log err and return

* Revert "don't throw exception,log err and return"

This reverts commit 6e1ba4a1bd82de3d4d07db5238a82184fd2ec9b4.

* add lock file name in exception

* cleanup  exception text

* use lock_guard in function put

* anonymous namespace

* use do-while(0) instead of goto

* cleanup read_db_all

* cleanup FileLock

* remove fchmod 0666 for apks

* Revert "remove fchmod 0666 for apks"

This reverts commit b389c31762417c4465d73be2453efcf7bc693aee.

* log instead of runtime error

* keep libs

Co-authored-by: deanlee <deanlee3@gmail.com>
albatross
Willem Melching 2021-04-09 16:35:44 +02:00 committed by GitHub
parent 9790c6d0b6
commit 74aa6e29c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 87 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
common/SConscript
View File

@ -1,4 +1,4 @@
Import('envCython')
Import('envCython', 'common')
envCython.Program('clock.so', 'clock.pyx')
envCython.Program('params_pyx.so', 'params_pyx.pyx')
envCython.Program('params_pyx.so', 'params_pyx.pyx', LIBS=envCython['LIBS'] + [common, 'zmq'])

205
selfdrive/common/params.cc
View File

@ -10,12 +10,25 @@
#include <dirent.h>
#include <sys/file.h>
#include <sys/stat.h>
#include <mutex>
#include <csignal>
#include <string.h>
#include "common/util.h"
#include "common/swaglog.h"
// keep trying if x gets interrupted by a signal
#define HANDLE_EINTR(x) \
({ \
decltype(x) ret; \
int try_cnt = 0; \
do { \
ret = (x); \
} while (ret == -1 && errno == EINTR && try_cnt++ < 100); \
ret; \
})
namespace {
#if defined(QCOM) || defined(QCOM2)
const std::string default_params_path = "/data/params";
@ -35,8 +48,8 @@ void params_sig_handler(int signal) {
params_do_exit = 1;
}
static int fsync_dir(const char* path){
int fd = open(path, O_RDONLY, 0755);
int fsync_dir(const char* path){
int fd = HANDLE_EINTR(open(path, O_RDONLY, 0755));
if (fd < 0){
return -1;
}
@ -50,7 +63,7 @@ static int fsync_dir(const char* path){
}
// TODO: replace by std::filesystem::create_directories
static int mkdir_p(std::string path) {
int mkdir_p(std::string path) {
char * _path = (char *)path.c_str();
mode_t prev_mask = umask(0);
@ -71,7 +84,7 @@ static int mkdir_p(std::string path) {
return 0;
}
static bool ensure_params_path(const std::string &param_path, const std::string &key_path) {
bool ensure_params_path(const std::string &param_path, const std::string &key_path) {
// Make sure params path exists
if (!util::file_exists(param_path) && mkdir_p(param_path) != 0) {
return false;
@ -110,6 +123,31 @@ static bool ensure_params_path(const std::string &param_path, const std::string
return chmod(key_path.c_str(), 0777) == 0;
}
class FileLock {
public:
FileLock(const std::string& file_name, int op) : fn_(file_name), op_(op) {}
void lock() {
fd_ = HANDLE_EINTR(open(fn_.c_str(), O_CREAT, 0775));
if (fd_ < 0) {
LOGE("Failed to open lock file %s, errno=%d", fn_.c_str(), errno);
return;
}
if (HANDLE_EINTR(flock(fd_, op_)) < 0) {
close(fd_);
LOGE("Failed to lock file %s, errno=%d", fn_.c_str(), errno);
}
}
void unlock() { close(fd_); }
private:
int fd_ = -1, op_;
std::string fn_;
};
} // namespace
Params::Params(bool persistent_param) : Params(persistent_param ? persistent_params_path : default_params_path) {}
Params::Params(const std::string &path) : params_path(path) {
@ -126,110 +164,54 @@ int Params::put(const char* key, const char* value, size_t value_size) {
// 4) rename the temp file to the real name
// 5) fsync() the containing directory
int lock_fd = -1;
int tmp_fd = -1;
int result;
std::string path;
std::string tmp_path;
ssize_t bytes_written;
std::string tmp_path = params_path + "/.tmp_value_XXXXXX";
int tmp_fd = mkstemp((char*)tmp_path.c_str());
if (tmp_fd < 0) return -1;
// Write value to temp.
tmp_path = params_path + "/.tmp_value_XXXXXX";
tmp_fd = mkstemp((char*)tmp_path.c_str());
bytes_written = write(tmp_fd, value, value_size);
if (bytes_written < 0 || (size_t)bytes_written != value_size) {
result = -20;
goto cleanup;
}
// Build lock path
path = params_path + "/.lock";
lock_fd = open(path.c_str(), O_CREAT, 0775);
// Build key path
path = params_path + "/d/" + std::string(key);
// Take lock.
result = flock(lock_fd, LOCK_EX);
if (result < 0) {
goto cleanup;
}
// change permissions to 0666 for apks
result = fchmod(tmp_fd, 0666);
if (result < 0) {
goto cleanup;
}
// fsync to force persist the changes.
result = fsync(tmp_fd);
if (result < 0) {
goto cleanup;
}
// Move temp into place.
result = rename(tmp_path.c_str(), path.c_str());
if (result < 0) {
goto cleanup;
}
// fsync parent directory
path = params_path + "/d";
result = fsync_dir(path.c_str());
if (result < 0) {
goto cleanup;
}
cleanup:
// Release lock.
if (lock_fd >= 0) {
close(lock_fd);
}
if (tmp_fd >= 0) {
if (result < 0) {
remove(tmp_path.c_str());
int result = -1;
do {
// Write value to temp.
ssize_t bytes_written = HANDLE_EINTR(write(tmp_fd, value, value_size));
if (bytes_written < 0 || (size_t)bytes_written != value_size) {
result = -20;
break;
}
close(tmp_fd);
}
// change permissions to 0666 for apks
if ((result = fchmod(tmp_fd, 0666)) < 0) break;
// fsync to force persist the changes.
if ((result = fsync(tmp_fd)) < 0) break;
FileLock file_lock(params_path + "/.lock", LOCK_EX);
std::lock_guard<FileLock> lk(file_lock);
// Move temp into place.
std::string path = params_path + "/d/" + std::string(key);
if ((result = rename(tmp_path.c_str(), path.c_str())) < 0) break;
// fsync parent directory
path = params_path + "/d";
result = fsync_dir(path.c_str());
} while(0);
close(tmp_fd);
remove(tmp_path.c_str());
return result;
}
int Params::remove(const char *key) {
int lock_fd = -1;
int result;
std::string path;
// Build lock path, and open lockfile
path = params_path + "/.lock";
lock_fd = open(path.c_str(), O_CREAT, 0775);
// Take lock.
result = flock(lock_fd, LOCK_EX);
if (result < 0) {
goto cleanup;
}
FileLock file_lock(params_path + "/.lock", LOCK_EX);
std::lock_guard<FileLock> lk(file_lock);
// Delete value.
path = params_path + "/d/" + key;
result = ::remove(path.c_str());
std::string path = params_path + "/d/" + key;
int result = ::remove(path.c_str());
if (result != 0) {
result = ERR_NO_VALUE;
goto cleanup;
return result;
}
// fsync parent directory
path = params_path + "/d";
result = fsync_dir(path.c_str());
if (result < 0) {
goto cleanup;
}
cleanup:
// Release lock.
if (lock_fd >= 0) {
close(lock_fd);
}
return result;
return fsync_dir(path.c_str());
}
std::string Params::get(const char *key, bool block) {
@ -257,37 +239,20 @@ std::string Params::get(const char *key, bool block) {
}
int Params::read_db_all(std::map<std::string, std::string> *params) {
int err = 0;
std::string lock_path = params_path + "/.lock";
int lock_fd = open(lock_path.c_str(), 0);
if (lock_fd < 0) return -1;
err = flock(lock_fd, LOCK_SH);
if (err < 0) {
close(lock_fd);
return err;
}
FileLock file_lock(params_path + "/.lock", LOCK_SH);
std::lock_guard<FileLock> lk(file_lock);
std::string key_path = params_path + "/d";
DIR *d = opendir(key_path.c_str());
if (!d) {
close(lock_fd);
return -1;
}
if (!d) return -1;
struct dirent *de = NULL;
while ((de = readdir(d))) {
if (!isalnum(de->d_name[0])) continue;
std::string key = std::string(de->d_name);
std::string value = util::read_file(key_path + "/" + key);
(*params)[key] = value;
if (isalnum(de->d_name[0])) {
(*params)[de->d_name] = util::read_file(key_path + "/" + de->d_name);
}
}
closedir(d);
close(lock_fd);
return 0;
}