Firmware updater: Fix a race condition

avrdude: Handle OOM with configurable handler
This commit is contained in:
Vojtech Kral 2018-08-07 11:28:39 +02:00 committed by bubnikv
parent bd667aad6e
commit 60a0375ff9
14 changed files with 193 additions and 67 deletions

View File

@ -2,6 +2,10 @@
#include <deque> #include <deque>
#include <thread> #include <thread>
#include <cstring>
#include <cstdlib>
#include <new>
#include <exception>
extern "C" { extern "C" {
#include "ac_cfg.h" #include "ac_cfg.h"
@ -28,6 +32,11 @@ static void avrdude_progress_handler_closure(const char *task, unsigned progress
(*progress_fn)(task, progress); (*progress_fn)(task, progress);
} }
static void avrdude_oom_handler(const char *context, void *user_p)
{
throw std::bad_alloc();
}
// Private // Private
@ -47,16 +56,22 @@ struct AvrDude::priv
priv(std::string &&sys_config) : sys_config(sys_config) {} priv(std::string &&sys_config) : sys_config(sys_config) {}
void set_handlers();
void unset_handlers();
int run_one(const std::vector<std::string> &args); int run_one(const std::vector<std::string> &args);
int run(); int run();
struct HandlerGuard
{
priv &p;
HandlerGuard(priv &p) : p(p) { p.set_handlers(); }
~HandlerGuard() { p.unset_handlers(); }
};
}; };
int AvrDude::priv::run_one(const std::vector<std::string> &args) { void AvrDude::priv::set_handlers()
std::vector<char*> c_args {{ const_cast<char*>(PACKAGE_NAME) }}; {
for (const auto &arg : args) {
c_args.push_back(const_cast<char*>(arg.data()));
}
if (message_fn) { if (message_fn) {
::avrdude_message_handler_set(avrdude_message_handler_closure, reinterpret_cast<void*>(&message_fn)); ::avrdude_message_handler_set(avrdude_message_handler_closure, reinterpret_cast<void*>(&message_fn));
} else { } else {
@ -69,10 +84,27 @@ int AvrDude::priv::run_one(const std::vector<std::string> &args) {
::avrdude_progress_handler_set(nullptr, nullptr); ::avrdude_progress_handler_set(nullptr, nullptr);
} }
const auto res = ::avrdude_main(static_cast<int>(c_args.size()), c_args.data(), sys_config.c_str()); ::avrdude_oom_handler_set(avrdude_oom_handler, nullptr);
}
void AvrDude::priv::unset_handlers()
{
::avrdude_message_handler_set(nullptr, nullptr); ::avrdude_message_handler_set(nullptr, nullptr);
::avrdude_progress_handler_set(nullptr, nullptr); ::avrdude_progress_handler_set(nullptr, nullptr);
::avrdude_oom_handler_set(nullptr, nullptr);
}
int AvrDude::priv::run_one(const std::vector<std::string> &args) {
std::vector<char*> c_args {{ const_cast<char*>(PACKAGE_NAME) }};
for (const auto &arg : args) {
c_args.push_back(const_cast<char*>(arg.data()));
}
HandlerGuard guard(*this);
const auto res = ::avrdude_main(static_cast<int>(c_args.size()), c_args.data(), sys_config.c_str());
return res; return res;
} }
@ -134,7 +166,7 @@ AvrDude& AvrDude::on_complete(CompleteFn fn)
int AvrDude::run_sync() int AvrDude::run_sync()
{ {
return p->run(); return p ? p->run() : -1;
} }
AvrDude::Ptr AvrDude::run() AvrDude::Ptr AvrDude::run()
@ -143,19 +175,46 @@ AvrDude::Ptr AvrDude::run()
if (self->p) { if (self->p) {
auto avrdude_thread = std::thread([self]() { auto avrdude_thread = std::thread([self]() {
bool cancel = false; try {
int res = -1; if (self->p->run_fn) {
self->p->run_fn(self);
}
if (self->p->run_fn) { if (! self->p->cancelled) {
self->p->run_fn(); self->p->exit_code = self->p->run();
} }
if (! self->p->cancelled) { if (self->p->complete_fn) {
self->p->exit_code = self->p->run(); self->p->complete_fn();
} }
} catch (const std::exception &ex) {
self->p->exit_code = EXIT_EXCEPTION;
if (self->p->complete_fn) { static const char *msg = "An exception was thrown in the background thread:\n";
self->p->complete_fn();
const char *what = ex.what();
auto &message_fn = self->p->message_fn;
if (message_fn) {
message_fn(msg, sizeof(msg));
message_fn(what, std::strlen(what));
message_fn("\n", 1);
}
if (self->p->complete_fn) {
self->p->complete_fn();
}
} catch (...) {
self->p->exit_code = EXIT_EXCEPTION;
static const char *msg = "An unkown exception was thrown in the background thread.\n";
if (self->p->message_fn) {
self->p->message_fn(msg, sizeof(msg));
}
if (self->p->complete_fn) {
self->p->complete_fn();
}
} }
}); });

View File

@ -11,8 +11,13 @@ namespace Slic3r {
class AvrDude class AvrDude
{ {
public: public:
enum {
EXIT_SUCCEESS = 0,
EXIT_EXCEPTION = -1000,
};
typedef std::shared_ptr<AvrDude> Ptr; typedef std::shared_ptr<AvrDude> Ptr;
typedef std::function<void()> RunFn; typedef std::function<void(Ptr /* avrdude */)> RunFn;
typedef std::function<void(const char * /* msg */, unsigned /* size */)> MessageFn; typedef std::function<void(const char * /* msg */, unsigned /* size */)> MessageFn;
typedef std::function<void(const char * /* task */, unsigned /* progress */)> ProgressFn; typedef std::function<void(const char * /* task */, unsigned /* progress */)> ProgressFn;
typedef std::function<void()> CompleteFn; typedef std::function<void()> CompleteFn;
@ -49,10 +54,18 @@ public:
// This has no effect when using run_sync(). // This has no effect when using run_sync().
AvrDude& on_complete(CompleteFn fn); AvrDude& on_complete(CompleteFn fn);
// Perform AvrDude invocation(s) synchronously on the current thread
int run_sync(); int run_sync();
// Perform AvrDude invocation(s) on a background thread.
// Current instance is moved into a shared_ptr which is returned (and also passed in on_run, if any).
Ptr run(); Ptr run();
// Cancel current operation
void cancel(); void cancel();
// If there is a background thread and it is joinable, join() it,
// that is, wait for it to finish.
void join(); void join();
bool cancelled(); // Whether avrdude run was cancelled bool cancelled(); // Whether avrdude run was cancelled

View File

@ -39,6 +39,12 @@ typedef void (*avrdude_progress_handler_t)(const char *task, unsigned progress,
void avrdude_progress_handler_set(avrdude_progress_handler_t newhandler, void *user_p); void avrdude_progress_handler_set(avrdude_progress_handler_t newhandler, void *user_p);
void avrdude_progress_external(const char *task, unsigned progress); void avrdude_progress_external(const char *task, unsigned progress);
// OOM handler
typedef void (*avrdude_oom_handler_t)(const char *context, void *user_p);
void avrdude_oom_handler_set(avrdude_oom_handler_t newhandler, void *user_p);
void avrdude_oom(const char *context);
// Cancellation // Cancellation
void avrdude_cancel(); void avrdude_cancel();

View File

@ -36,8 +36,9 @@ OPCODE * avr_new_opcode(void)
m = (OPCODE *)malloc(sizeof(*m)); m = (OPCODE *)malloc(sizeof(*m));
if (m == NULL) { if (m == NULL) {
avrdude_message(MSG_INFO, "avr_new_opcode(): out of memory\n"); // avrdude_message(MSG_INFO, "avr_new_opcode(): out of memory\n");
exit(1); // exit(1);
avrdude_oom("avr_new_opcode(): out of memory\n");
} }
memset(m, 0, sizeof(*m)); memset(m, 0, sizeof(*m));
@ -56,8 +57,9 @@ static OPCODE * avr_dup_opcode(OPCODE * op)
m = (OPCODE *)malloc(sizeof(*m)); m = (OPCODE *)malloc(sizeof(*m));
if (m == NULL) { if (m == NULL) {
avrdude_message(MSG_INFO, "avr_dup_opcode(): out of memory\n"); // avrdude_message(MSG_INFO, "avr_dup_opcode(): out of memory\n");
exit(1); // exit(1);
avrdude_oom("avr_dup_opcode(): out of memory\n");
} }
memcpy(m, op, sizeof(*m)); memcpy(m, op, sizeof(*m));
@ -249,8 +251,9 @@ AVRMEM * avr_new_memtype(void)
m = (AVRMEM *)malloc(sizeof(*m)); m = (AVRMEM *)malloc(sizeof(*m));
if (m == NULL) { if (m == NULL) {
avrdude_message(MSG_INFO, "avr_new_memtype(): out of memory\n"); // avrdude_message(MSG_INFO, "avr_new_memtype(): out of memory\n");
exit(1); // exit(1);
avrdude_oom("avr_new_memtype(): out of memory\n");
} }
memset(m, 0, sizeof(*m)); memset(m, 0, sizeof(*m));
@ -300,9 +303,10 @@ AVRMEM * avr_dup_mem(AVRMEM * m)
if (m->buf != NULL) { if (m->buf != NULL) {
n->buf = (unsigned char *)malloc(n->size); n->buf = (unsigned char *)malloc(n->size);
if (n->buf == NULL) { if (n->buf == NULL) {
avrdude_message(MSG_INFO, "avr_dup_mem(): out of memory (memsize=%d)\n", // avrdude_message(MSG_INFO, "avr_dup_mem(): out of memory (memsize=%d)\n",
n->size); // n->size);
exit(1); // exit(1);
avrdude_oom("avr_dup_mem(): out of memory");
} }
memcpy(n->buf, m->buf, n->size); memcpy(n->buf, m->buf, n->size);
} }
@ -310,9 +314,10 @@ AVRMEM * avr_dup_mem(AVRMEM * m)
if (m->tags != NULL) { if (m->tags != NULL) {
n->tags = (unsigned char *)malloc(n->size); n->tags = (unsigned char *)malloc(n->size);
if (n->tags == NULL) { if (n->tags == NULL) {
avrdude_message(MSG_INFO, "avr_dup_mem(): out of memory (memsize=%d)\n", // avrdude_message(MSG_INFO, "avr_dup_mem(): out of memory (memsize=%d)\n",
n->size); // n->size);
exit(1); // exit(1);
avrdude_oom("avr_dup_mem(): out of memory");
} }
memcpy(n->tags, m->tags, n->size); memcpy(n->tags, m->tags, n->size);
} }
@ -441,8 +446,9 @@ AVRPART * avr_new_part(void)
p = (AVRPART *)malloc(sizeof(AVRPART)); p = (AVRPART *)malloc(sizeof(AVRPART));
if (p == NULL) { if (p == NULL) {
avrdude_message(MSG_INFO, "new_part(): out of memory\n"); // avrdude_message(MSG_INFO, "new_part(): out of memory\n");
exit(1); // exit(1);
avrdude_oom("new_part(): out of memory\n");
} }
memset(p, 0, sizeof(*p)); memset(p, 0, sizeof(*p));

View File

@ -1135,9 +1135,10 @@ static void buspirate_setup(struct programmer_t *pgm)
{ {
/* Allocate private data */ /* Allocate private data */
if ((pgm->cookie = calloc(1, sizeof(struct pdata))) == 0) { if ((pgm->cookie = calloc(1, sizeof(struct pdata))) == 0) {
avrdude_message(MSG_INFO, "%s: buspirate_initpgm(): Out of memory allocating private data\n", // avrdude_message(MSG_INFO, "%s: buspirate_initpgm(): Out of memory allocating private data\n",
progname); // progname);
exit(1); // exit(1);
avrdude_oom("buspirate_initpgm(): Out of memory allocating private data\n");
} }
PDATA(pgm)->serial_recv_timeout = 100; PDATA(pgm)->serial_recv_timeout = 100;
} }

View File

@ -63,9 +63,10 @@ struct pdata
static void butterfly_setup(PROGRAMMER * pgm) static void butterfly_setup(PROGRAMMER * pgm)
{ {
if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) { if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) {
avrdude_message(MSG_INFO, "%s: butterfly_setup(): Out of memory allocating private data\n", // avrdude_message(MSG_INFO, "%s: butterfly_setup(): Out of memory allocating private data\n",
progname); // progname);
exit(1); // exit(1);
avrdude_oom("butterfly_setup(): Out of memory allocating private data\n");
} }
memset(pgm->cookie, 0, sizeof(struct pdata)); memset(pgm->cookie, 0, sizeof(struct pdata));
} }

View File

@ -2834,7 +2834,8 @@ YY_BUFFER_STATE yy_scan_bytes (const char * yybytes, int _yybytes_len )
n = (yy_size_t) (_yybytes_len + 2); n = (yy_size_t) (_yybytes_len + 2);
buf = (char *) yyalloc( n ); buf = (char *) yyalloc( n );
if ( ! buf ) if ( ! buf )
YY_FATAL_ERROR( "out of dynamic memory in yy_scan_bytes()" ); // YY_FATAL_ERROR( "out of dynamic memory in yy_scan_bytes()" );
avrdude_oom("out of dynamic memory in yy_scan_bytes()");
for ( i = 0; i < _yybytes_len; ++i ) for ( i = 0; i < _yybytes_len; ++i )
buf[i] = yybytes[i]; buf[i] = yybytes[i];
@ -2859,8 +2860,9 @@ YY_BUFFER_STATE yy_scan_bytes (const char * yybytes, int _yybytes_len )
static void yynoreturn yy_fatal_error (const char* msg ) static void yynoreturn yy_fatal_error (const char* msg )
{ {
fprintf( stderr, "%s\n", msg ); fprintf( stderr, "%s\n", msg );
exit( YY_EXIT_FAILURE ); // exit( YY_EXIT_FAILURE );
abort();
} }
/* Redefine yyless() so it works in section 3 code. */ /* Redefine yyless() so it works in section 3 code. */

View File

@ -144,6 +144,33 @@ void avrdude_progress_external(const char *task, unsigned progress)
avrdude_progress_handler(task, progress, avrdude_progress_handler_user_p); avrdude_progress_handler(task, progress, avrdude_progress_handler_user_p);
} }
static void avrdude_oom_handler_null(const char *context, void *user_p)
{
// Output a message and just exit
fputs("avrdude: Out of memory: ", stderr);
fputs(context, stderr);
exit(99);
}
static void *avrdude_oom_handler_user_p = NULL;
static avrdude_oom_handler_t avrdude_oom_handler = avrdude_oom_handler_null;
void avrdude_oom_handler_set(avrdude_oom_handler_t newhandler, void *user_p)
{
if (newhandler != NULL) {
avrdude_oom_handler = newhandler;
avrdude_oom_handler_user_p = user_p;
} else {
avrdude_oom_handler = avrdude_oom_handler_null;
avrdude_oom_handler_user_p = NULL;
}
}
void avrdude_oom(const char *context)
{
avrdude_oom_handler(context, avrdude_oom_handler_user_p);
}
void avrdude_cancel() void avrdude_cancel()
{ {
cancel_flag = true; cancel_flag = true;

View File

@ -172,9 +172,10 @@ PROGRAMMER * pgm_dup(const PROGRAMMER * const src)
for (ln = lfirst(src->usbpid); ln; ln = lnext(ln)) { for (ln = lfirst(src->usbpid); ln; ln = lnext(ln)) {
int *ip = malloc(sizeof(int)); int *ip = malloc(sizeof(int));
if (ip == NULL) { if (ip == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory allocating programmer structure\n", // avrdude_message(MSG_INFO, "%s: out of memory allocating programmer structure\n",
progname); // progname);
exit(1); // exit(1);
avrdude_oom("out of memory allocating programmer structure\n");
} }
*ip = *(int *) ldata(ln); *ip = *(int *) ldata(ln);
ladd(pgm->usbpid, ip); ladd(pgm->usbpid, ip);

View File

@ -246,10 +246,11 @@ static int ser_open(char * port, union pinfo pinfo, union filedescriptor *fdp)
newname = malloc(strlen("\\\\.\\") + strlen(port) + 1); newname = malloc(strlen("\\\\.\\") + strlen(port) + 1);
if (newname == 0) { if (newname == 0) {
avrdude_message(MSG_INFO, "%s: ser_open(): out of memory\n", // avrdude_message(MSG_INFO, "%s: ser_open(): out of memory\n",
progname); // progname);
exit(1); // exit(1);
} avrdude_oom("ser_open(): out of memory\n");
}
strcpy(newname, "\\\\.\\"); strcpy(newname, "\\\\.\\");
strcat(newname, port); strcat(newname, port);

View File

@ -295,9 +295,10 @@ static int stk600_xprog_program_enable(PROGRAMMER * pgm, AVRPART * p);
void stk500v2_setup(PROGRAMMER * pgm) void stk500v2_setup(PROGRAMMER * pgm)
{ {
if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) { if ((pgm->cookie = malloc(sizeof(struct pdata))) == 0) {
avrdude_message(MSG_INFO, "%s: stk500v2_setup(): Out of memory allocating private data\n", // avrdude_message(MSG_INFO, "%s: stk500v2_setup(): Out of memory allocating private data\n",
progname); // progname);
exit(1); // exit(1);
avrdude_oom("stk500v2_setup(): Out of memory allocating private data\n");
} }
memset(pgm->cookie, 0, sizeof(struct pdata)); memset(pgm->cookie, 0, sizeof(struct pdata));
PDATA(pgm)->command_sequence = 1; PDATA(pgm)->command_sequence = 1;

View File

@ -38,8 +38,9 @@ UPDATE * parse_op(char * s)
upd = (UPDATE *)malloc(sizeof(UPDATE)); upd = (UPDATE *)malloc(sizeof(UPDATE));
if (upd == NULL) { if (upd == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory\n", progname); // avrdude_message(MSG_INFO, "%s: out of memory\n", progname);
exit(1); // exit(1);
avrdude_oom("parse_op: out of memory\n");
} }
i = 0; i = 0;
@ -53,8 +54,9 @@ UPDATE * parse_op(char * s)
upd->op = DEVICE_WRITE; upd->op = DEVICE_WRITE;
upd->filename = (char *)malloc(strlen(buf) + 1); upd->filename = (char *)malloc(strlen(buf) + 1);
if (upd->filename == NULL) { if (upd->filename == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory\n", progname); // avrdude_message(MSG_INFO, "%s: out of memory\n", progname);
exit(1); // exit(1);
avrdude_oom("parse_op: out of memory\n");
} }
strcpy(upd->filename, buf); strcpy(upd->filename, buf);
upd->format = FMT_AUTO; upd->format = FMT_AUTO;
@ -63,8 +65,9 @@ UPDATE * parse_op(char * s)
upd->memtype = (char *)malloc(strlen(buf)+1); upd->memtype = (char *)malloc(strlen(buf)+1);
if (upd->memtype == NULL) { if (upd->memtype == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory\n", progname); // avrdude_message(MSG_INFO, "%s: out of memory\n", progname);
exit(1); // exit(1);
avrdude_oom("parse_op: out of memory\n");
} }
strcpy(upd->memtype, buf); strcpy(upd->memtype, buf);
@ -179,8 +182,9 @@ UPDATE * dup_update(UPDATE * upd)
u = (UPDATE *)malloc(sizeof(UPDATE)); u = (UPDATE *)malloc(sizeof(UPDATE));
if (u == NULL) { if (u == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory\n", progname); // avrdude_message(MSG_INFO, "%s: out of memory\n", progname);
exit(1); // exit(1);
avrdude_oom("dup_update: out of memory\n");
} }
memcpy(u, upd, sizeof(UPDATE)); memcpy(u, upd, sizeof(UPDATE));
@ -200,8 +204,9 @@ UPDATE * new_update(int op, char * memtype, int filefmt, char * filename, unsign
u = (UPDATE *)malloc(sizeof(UPDATE)); u = (UPDATE *)malloc(sizeof(UPDATE));
if (u == NULL) { if (u == NULL) {
avrdude_message(MSG_INFO, "%s: out of memory\n", progname); // avrdude_message(MSG_INFO, "%s: out of memory\n", progname);
exit(1); // exit(1);
avrdude_oom("new_update: out of memory\n");
} }
u->memtype = strdup(memtype); u->memtype = strdup(memtype);

View File

@ -85,9 +85,10 @@ static void wiring_setup(PROGRAMMER * pgm)
* Now prepare our data * Now prepare our data
*/ */
if ((mycookie = malloc(sizeof(struct wiringpdata))) == 0) { if ((mycookie = malloc(sizeof(struct wiringpdata))) == 0) {
avrdude_message(MSG_INFO, "%s: wiring_setup(): Out of memory allocating private data\n", // avrdude_message(MSG_INFO, "%s: wiring_setup(): Out of memory allocating private data\n",
progname); // progname);
exit(1); // exit(1);
avrdude_oom("wiring_setup(): Out of memory allocating private data\n");
} }
memset(mycookie, 0, sizeof(struct wiringpdata)); memset(mycookie, 0, sizeof(struct wiringpdata));
WIRINGPDATA(mycookie)->snoozetime = 0; WIRINGPDATA(mycookie)->snoozetime = 0;

View File

@ -551,8 +551,10 @@ void FirmwareDialog::priv::perform_upload()
// because the dialog ensures it doesn't exit before the background thread is done. // because the dialog ensures it doesn't exit before the background thread is done.
auto q = this->q; auto q = this->q;
this->avrdude = avrdude avrdude
.on_run([this]() { .on_run([this](AvrDude::Ptr avrdude) {
this->avrdude = std::move(avrdude);
try { try {
switch (this->hex_file.device) { switch (this->hex_file.device) {
case HexFile::DEV_MK3: case HexFile::DEV_MK3: