2

I'm trying to expose a C interface for my C++ library. This notably involve functions that allow the user to create, launch, query the status, then release a background task.

The task is implemented within a C++ class, which members are protected from concurrent read/write via an std::mutex.

My issue comes when I expose a C interface for this background task. Basically I have say the following functions (assuming task_t is an opaque pointer to an actual struct containing the real task class):

task_t* mylib_task_create();
bool mylib_task_is_running(task_t* task);
void mylib_task_release(task_t* task);

My goal is to make any concurrent usage of these functions thread-safe, however I'm not sure exactly how, i.e. that if a client code thread calls mylib_task_is_running() at the same time that another thread calls mylib_task_release(), then everything's fine.

At first I thought about adding an std::mutex to the implementation of task_t, but that means the delete statement at the end of mylib_task_release() will have to happen while the mutex is not held, which means it doesn't completely solve the problem.

I also thought about using some sort of reference counting but I still end up against the same kind of issue where the actual delete might happen right after a hypothetical retain() function is called.

I feel like there should be a (relatively) simple solution to this but I can't quite put my hand on it. How can I make it so I don't have to force the client code to protect accesses to task_t?

JBL
  • 12,588
  • 4
  • 53
  • 84
  • Can't you just acquire the `task_t` mutex in `mylib_task_release()`? – 500 - Internal Server Error Feb 09 '21 at 14:41
  • Well the issue is, if the mutex is a member of the task_t structure, then I need to delete the task while the mutex is locked, and as per the [reference](https://en.cppreference.com/w/cpp/thread/mutex/%7Emutex), `The behavior is undefined if the mutex is owned by any thread or if any thread terminates while holding any ownership of the mutex. ` – JBL Feb 09 '21 at 14:44
  • wouldn't a shared_pointer solve this problem? – Devolus Feb 09 '21 at 14:48
  • @Devolus How so? – JBL Feb 09 '21 at 14:53
  • When you delete the shared pointer it is only deleted when all references are done. – Devolus Feb 09 '21 at 14:58
  • As far as I can tell, this would only move the issue to the `shared_ptr` itself and would be similar to my first idea of having a mutex inside `task_t`. Yes what's pointed at by the `shared_ptr` would be alright, but I still need to return a C pointer, which I eventually need to delete. Were you thinking of something else? Or am I mistaken somewhere? – JBL Feb 09 '21 at 15:05
  • @rustyx Making this C API thread-safe is my actual goal. Indeed `task_t` gets deleted in `mylib_task_release()`, and all the function receiving a `task_t*` parameter check that it's non-null so once it's deleted, these functions are fine. The issue is if they manage to slip in the null check right before it's deleted by another thread. – JBL Feb 09 '21 at 15:13
  • Can you post a bit more of your `task_t`? And, some of the representative access functions? You can have a per-task mutex to control access to the individual task data. If you're adding to or deleting from the list of tasks, you can have a separate global mutex for that (it could be part of the _list_ control struct). Some per-task access can be done with atomics or just cache sync/flush. I've done a zillion of these impl, but a lot depends on specifics. – Craig Estey Feb 09 '21 at 17:36
  • 'mylib_task_is_running' is almost certainly going to be unreliable anyway. An 'onTaskCompletion()' callback would seem to be actually useful:) – Martin James Feb 09 '21 at 18:15
  • @CraigEstey The mutex per task is kinda what I wanted to achieve first, but sticking it within the task seems like a no-no, and I wasn't sure where else I could store it without using a global. – JBL Feb 10 '21 at 09:56
  • You can use both types of locks. The global/list lock is for insertions/deletions from the list (e.g. for a doubly linked list, adjusting head/tail and next/prev). The per-task is atomicity with changes to a task block. You can use RCU for insert/delete: https://en.wikipedia.org/wiki/Read-copy-update. Also, `stdatomic.h` can be used for per-task updates. Break it down: In `task_t`, what is local/bookkeeping for the task and what is status that is intertask? Local is ordinary/non-lock access. Intertask can use `stdatomic` (e.g. `atomic_compare_exchange_strong`) and no lock needed – Craig Estey Feb 10 '21 at 20:40
  • For the global lock for list insert/delete/traversal, `pthread_rwlock_t` may be a better fit. Readers that need to traverse the list can use `pthread_rwlock_rdlock` (this allows _multiple_ simultaneous readers and _no_ insert/delete). Insert/delete can use `pthread_rwlock_wrlock` (this allows _one_ inserter/deleter and _no_ readers). – Craig Estey Feb 10 '21 at 20:46
  • Instead of a [global] mutex, consider using a "ticket lock": https://en.wikipedia.org/wiki/Ticket_lock I've used that [I "reinvented" it ;-) and then found that was already a known thing]. With `stdatomic`. It promotes "fairness": No high priority task can starve a lower priority task from a resource. You could implement all this in stages: Start with _one_ global lock and evolve the locks to finer grain/lockless as you gain experience – Craig Estey Feb 10 '21 at 20:53
  • @CraigEstey Thanks, I think it's getting clearer in terms of what I'm eventually going to have to do, and that I cannot just rely on these functions with their current signature, as well as the task itself, to carve a solution out of just that. – JBL Feb 11 '21 at 12:59
  • 1
    Well, good luck. I almost forgot. Here's a simple/crude task block example that I put together as an answer: https://stackoverflow.com/questions/64324334/how-to-share-variables-among-threads/64326906#64326906 Also, the linux kernel source would be a good resource – Craig Estey Feb 11 '21 at 16:19

1 Answers1

0

if task_t is being deleted, you should ensure that nobody else has a pointer to it.

if one thread is deleting task_t and the other is trying to acquire it's mutex, it should be apparent that you should not have deleted the task_t.

shared_ptrs are a great help for this.

N. Prone
  • 180
  • 7
  • Thanks for providing an answer. I'm not sure how `shared_ptr` would help with what you describe since this is a C API, and how one would ensure that nobody else has a pointer to a task since this API returns the pointer and I'm not in control of what the client code does with this pointer. Does it make sense to call API functions on a task that's been released, well yes. But my main concern is that it at least doesn't crash. – JBL Feb 10 '21 at 10:06
  • I just re-read your question -- the hard case is "running() gets called while release() is supposed to release resources". This already indicates your mutex needs to be higher-up, and since you don't want to access a Task* after its been destroyed, it hints to me that you need to keep track of which tasks are invalid. You need a "TaskManager" class to perform synchronization and to ensure Task*'s are valid after acquiring the mutex. – N. Prone Feb 10 '21 at 21:00
  • Yes I'm coming to the realization that I need a higher-level mechanism to keep track of that and probably associate tasks with a mutex in a non-intrusive way. – JBL Feb 11 '21 at 12:56