Potential errors(e.g., deadlock) due to the incorrect lock usage

[jenny-cheung]

Hi, developers, thank you for your checking. The lock &gdb->client_mutex may be acquired twice, which can lead to deadlock? Also, it seems the lock &gdb->client_mutex is not released in the thread correctly because there is no cen64_mutex_unlock(&gdb->client_mutex); statement.

cen64/gdb/gdb.c

Line 85 in 1b31ca9

cen64_mutex_lock(&gdb->client_mutex);

cen64/gdb/gdb.c

Line 91 in 1b31ca9

cen64_mutex_lock(&gdb->client_mutex);

The relevant code is shown:

CEN64_THREAD_RETURN_TYPE gdb_thread(void *opaque) {
  cen64_thread_setname(NULL, "gdb");
  struct gdb *gdb = (struct gdb *) opaque;

  cen64_mutex_lock(&gdb->client_mutex); // acquire the lock first time

  // wait until first breakpoint is hit before entering loop
  if (gdb->flags & GDB_FLAGS_INITIAL) {
    cen64_cv_wait(&gdb->client_semaphore, &gdb->client_mutex);
  } else {
    cen64_mutex_lock(&gdb->client_mutex); // acquire the lock second time
  }

 ...;
  return CEN64_THREAD_RETURN_VAL;
}

cen64_cold bool gdb_init(struct gdb* gdb, struct cen64_device* device, const char* host) {
  ...;

  if (cen64_thread_create(&gdb->thread, gdb_thread, gdb)) { // thread call site
    cen64_mutex_destroy(&gdb->client_mutex);
    cen64_cv_destroy(&gdb->client_semaphore);
    printf("Failed to create gdb thread.\n");
    return false;
  }

  return true;
}

[lambertjamesd]

Have you been able to actually get a deadlock or just pointing out a potential lock based on looking at the code?

[jenny-cheung]

@lambertjamesd Thank you for your reply. Actually, I do not trigger the deadlock in runtime but I use a static code analysis tool that warns of this case.

[lambertjamesd]

I wrote that code initially. I don't remember why I wrote it that way. I'll see if I have some time tonight to fix it.

[jenny-cheung]

Thanks~