Fix file_handler process race condition
The current process could be waited both by run_file_handler() and file_handler_stop(). To avoid the race condition, wait the process without closing, then close with mutex locked.
This commit is contained in:
parent
6a50231698
commit
7afd149f4b
4 changed files with 18 additions and 12 deletions
|
@ -135,13 +135,13 @@ run_file_handler(void *data) {
|
||||||
mutex_unlock(file_handler->mutex);
|
mutex_unlock(file_handler->mutex);
|
||||||
|
|
||||||
if (req.action == ACTION_INSTALL_APK) {
|
if (req.action == ACTION_INSTALL_APK) {
|
||||||
if (process_check_success(process, "adb install")) {
|
if (process_check_success(process, "adb install", false)) {
|
||||||
LOGI("%s successfully installed", req.file);
|
LOGI("%s successfully installed", req.file);
|
||||||
} else {
|
} else {
|
||||||
LOGE("Failed to install %s", req.file);
|
LOGE("Failed to install %s", req.file);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
if (process_check_success(process, "adb push")) {
|
if (process_check_success(process, "adb push", false)) {
|
||||||
LOGI("%s successfully pushed to %s", req.file,
|
LOGI("%s successfully pushed to %s", req.file,
|
||||||
file_handler->push_target);
|
file_handler->push_target);
|
||||||
} else {
|
} else {
|
||||||
|
@ -150,6 +150,14 @@ run_file_handler(void *data) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
mutex_lock(file_handler->mutex);
|
||||||
|
// Close the process (it is necessary already terminated)
|
||||||
|
// Execute this call with mutex locked to avoid race conditions with
|
||||||
|
// file_handler_stop()
|
||||||
|
process_close(file_handler->current_process);
|
||||||
|
file_handler->current_process = PROCESS_NONE;
|
||||||
|
mutex_unlock(file_handler->mutex);
|
||||||
|
|
||||||
file_handler_request_destroy(&req);
|
file_handler_request_destroy(&req);
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
|
@ -178,8 +186,6 @@ file_handler_stop(struct file_handler *file_handler) {
|
||||||
if (!process_terminate(file_handler->current_process)) {
|
if (!process_terminate(file_handler->current_process)) {
|
||||||
LOGW("Could not terminate install process");
|
LOGW("Could not terminate install process");
|
||||||
}
|
}
|
||||||
process_wait(file_handler->current_process, true); // ignore exit code
|
|
||||||
file_handler->current_process = PROCESS_NONE;
|
|
||||||
}
|
}
|
||||||
mutex_unlock(file_handler->mutex);
|
mutex_unlock(file_handler->mutex);
|
||||||
}
|
}
|
||||||
|
|
|
@ -100,31 +100,31 @@ push_server(const char *serial) {
|
||||||
}
|
}
|
||||||
process_t process = adb_push(serial, server_path, DEVICE_SERVER_PATH);
|
process_t process = adb_push(serial, server_path, DEVICE_SERVER_PATH);
|
||||||
SDL_free(server_path);
|
SDL_free(server_path);
|
||||||
return process_check_success(process, "adb push");
|
return process_check_success(process, "adb push", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
enable_tunnel_reverse(const char *serial, uint16_t local_port) {
|
enable_tunnel_reverse(const char *serial, uint16_t local_port) {
|
||||||
process_t process = adb_reverse(serial, SOCKET_NAME, local_port);
|
process_t process = adb_reverse(serial, SOCKET_NAME, local_port);
|
||||||
return process_check_success(process, "adb reverse");
|
return process_check_success(process, "adb reverse", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
disable_tunnel_reverse(const char *serial) {
|
disable_tunnel_reverse(const char *serial) {
|
||||||
process_t process = adb_reverse_remove(serial, SOCKET_NAME);
|
process_t process = adb_reverse_remove(serial, SOCKET_NAME);
|
||||||
return process_check_success(process, "adb reverse --remove");
|
return process_check_success(process, "adb reverse --remove", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
enable_tunnel_forward(const char *serial, uint16_t local_port) {
|
enable_tunnel_forward(const char *serial, uint16_t local_port) {
|
||||||
process_t process = adb_forward(serial, local_port, SOCKET_NAME);
|
process_t process = adb_forward(serial, local_port, SOCKET_NAME);
|
||||||
return process_check_success(process, "adb forward");
|
return process_check_success(process, "adb forward", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
disable_tunnel_forward(const char *serial, uint16_t local_port) {
|
disable_tunnel_forward(const char *serial, uint16_t local_port) {
|
||||||
process_t process = adb_forward_remove(serial, local_port);
|
process_t process = adb_forward_remove(serial, local_port);
|
||||||
return process_check_success(process, "adb forward --remove");
|
return process_check_success(process, "adb forward --remove", true);
|
||||||
}
|
}
|
||||||
|
|
||||||
static bool
|
static bool
|
||||||
|
|
|
@ -3,12 +3,12 @@
|
||||||
#include "log.h"
|
#include "log.h"
|
||||||
|
|
||||||
bool
|
bool
|
||||||
process_check_success(process_t proc, const char *name) {
|
process_check_success(process_t proc, const char *name, bool close) {
|
||||||
if (proc == PROCESS_NONE) {
|
if (proc == PROCESS_NONE) {
|
||||||
LOGE("Could not execute \"%s\"", name);
|
LOGE("Could not execute \"%s\"", name);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
exit_code_t exit_code = process_wait(proc, true);
|
exit_code_t exit_code = process_wait(proc, close);
|
||||||
if (exit_code) {
|
if (exit_code) {
|
||||||
if (exit_code != NO_EXIT_CODE) {
|
if (exit_code != NO_EXIT_CODE) {
|
||||||
LOGE("\"%s\" returned with value %" PRIexitcode, name, exit_code);
|
LOGE("\"%s\" returned with value %" PRIexitcode, name, exit_code);
|
||||||
|
|
|
@ -61,7 +61,7 @@ process_close(process_t pid);
|
||||||
// convenience function to wait for a successful process execution
|
// convenience function to wait for a successful process execution
|
||||||
// automatically log process errors with the provided process name
|
// automatically log process errors with the provided process name
|
||||||
bool
|
bool
|
||||||
process_check_success(process_t proc, const char *name);
|
process_check_success(process_t proc, const char *name, bool close);
|
||||||
|
|
||||||
#ifndef _WIN32
|
#ifndef _WIN32
|
||||||
// only used to find package manager, not implemented for Windows
|
// only used to find package manager, not implemented for Windows
|
||||||
|
|
Loading…
Reference in a new issue