Skip to content

Commit

Permalink
async thread: cleanup thread id and state variable
Browse files Browse the repository at this point in the history
async_thread_id was set but never used,
start_ipc_thread returning -1 which isn't a valid pthread_t,
and handle used as a bool with a weird ++ pattern:
clean this all up by replacing handle with a proper state enum
(init, started, done), and join when we try to start thread again
after it's been done successfully
We still never join the thread when exiting after it's done once,
nor do we care about its pthread_exit return value, but at least
we don't leak resources everytime a new thread is started

Signed-off-by: Dominique Martinet <[email protected]>
Reviewed-by: Stefano babic <[email protected]>
  • Loading branch information
martinetd authored and sbabic committed May 9, 2022
1 parent 0027e8e commit 2f280b6
Showing 1 changed file with 23 additions and 12 deletions.
35 changes: 23 additions & 12 deletions ipc/network_ipc-if.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ struct async_lib {
terminated end;
};

static int handle = 0;
static enum async_thread_state {
ASYNC_THREAD_INIT,
ASYNC_THREAD_RUNNING,
ASYNC_THREAD_DONE
} running = ASYNC_THREAD_INIT;

static struct async_lib request;

Expand Down Expand Up @@ -83,7 +87,7 @@ static void *swupdate_async_thread(void *data)
}

out:
handle = 0;
running = ASYNC_THREAD_DONE;
if (rq->end)
rq->end((RECOVERY_STATUS)swupdate_result);

Expand All @@ -95,20 +99,21 @@ static void *swupdate_async_thread(void *data)
* to let build the ipc library without
* linking pctl code
*/
static pthread_t start_ipc_thread(void *(* start_routine) (void *), void *arg)
static void start_ipc_thread(void *(* start_routine) (void *), void *arg)
{
int ret;
pthread_t id;
pthread_attr_t attr;

pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

ret = pthread_create(&id, &attr, start_routine, arg);
ret = pthread_create(&async_thread_id, &attr, start_routine, arg);
if (ret) {
return -1;
perror("ipc thread creation failed");
return;
}
return id;

running = ASYNC_THREAD_RUNNING;
}

/*
Expand All @@ -121,8 +126,16 @@ int swupdate_async_start(writedata wr_func, getstatus status_func,
struct async_lib *rq;
int connfd;

if (handle)
switch (running) {
case ASYNC_THREAD_INIT:
break;
case ASYNC_THREAD_DONE:
pthread_join(async_thread_id, NULL);
running = ASYNC_THREAD_INIT;
break;
default:
return -EBUSY;
}

rq = get_request();

Expand All @@ -137,11 +150,9 @@ int swupdate_async_start(writedata wr_func, getstatus status_func,

rq->connfd = connfd;

async_thread_id = start_ipc_thread(swupdate_async_thread, rq);

handle++;
start_ipc_thread(swupdate_async_thread, rq);

return handle;
return running != ASYNC_THREAD_INIT;
}

int swupdate_image_write(char *buf, int size)
Expand Down

0 comments on commit 2f280b6

Please sign in to comment.