1

I'm trying to implement a tasking system that has the ability to make tasks destroy themselves if they run out of instructions. For this I thought of a mechanism in which I could pass back the unique_ptr handle returned by creating the task back to the task and have it run out of scope. I can't show you the task but imagine in the following example pass_back_ownership would somehow move the unique_ptr to the task function which destroys it if runs out of scope. Am I in UB-land or is this totally legit and good practice?

Demo

#include <future>
#include <memory>
#include <cstdio>

struct task;

using task_ptr = std::unique_ptr<task>;

struct task
{
    task() = default;

    ~task() {
        printf("task deleted!\n");
    }

    auto pass_back_ownership(task_ptr&& ptr) {
        auto tmp = std::move(ptr);
    }

    // Task stack
    // ...
};


int main()
{

    task_ptr ptr = std::make_unique<task>();

    ptr->pass_back_ownership(std::move(ptr));

    printf("end!\n");
}

Output:

task deleted!
end!
glades
  • 3,778
  • 1
  • 12
  • 34
  • 1
    Are we to tell you if w/e `// Destroy...` stands for is correct without seeing it? Maybe you handled it correctly, maybe not, how are we to tell? – StoryTeller - Unslander Monica Jan 14 '23 at 13:42
  • @StoryTeller-UnslanderMonica Well it destroys itself by running the unique_ptr out of scope. The comment is just there to annotate that. The question is if I'm good doing so from within the class method of the deleted object in question, as the object being destroyed will invalidate the this-ptr within the method (and Idk if "this" is still used in stack unwinding) – glades Jan 14 '23 at 13:46
  • You get a reference to a unique_ptr, that doesn't run anything out of scope. – StoryTeller - Unslander Monica Jan 14 '23 at 13:47
  • @StoryTeller-UnslanderMonica Yes it runs the loaded unique_ptr out of scope. – glades Jan 14 '23 at 13:49
  • Nothing will happen in the code you've posted, `std::move` is just a cast to a reference, no moves actually take place until you explicitly move the reference into another object – Alan Birtles Jan 14 '23 at 13:52
  • No, the one in main goes out of scope. The reference is just that, a reference. Try challenging your conclusions before adopting them. Printing additional checkpoints when testing is bare minimum. – StoryTeller - Unslander Monica Jan 14 '23 at 13:52
  • Does this answer your question? [Is "delete this" allowed in C++?](https://stackoverflow.com/questions/3150942/is-delete-this-allowed-in-c) Just a more convoluted way of doing this. – Richard Critten Jan 14 '23 at 13:52
  • @StoryTeller-UnslanderMonica You are of course right. I assume it got optimized out? When I move the ptr to a temporary inside scrope it now does what I wanted. – glades Jan 14 '23 at 13:57
  • @RichardCritten Yes indeed, along those lines. – glades Jan 14 '23 at 14:01
  • 1
    @glades In summary: as long you don't try and access the object after `delete this;` the code is well defined. – Richard Critten Jan 14 '23 at 14:04
  • 2
    Instead of moving to a local copy, I'd simply call `reset` on the pointer. If you want a unique pointer to run out of scope to delete the object though, I'd change the parameter type to `task_ptr ptr` instead which allows you to simply leave the body of `pass_back_ownership` empty, since the parameter gets destroyed at the end of the function call too... – fabian Jan 14 '23 at 14:07
  • @fabian That's going to be a problem with the form of the `ptr->pass_back_ownership(std::move(ptr))` expression though before C++17. So a bit of care must be taken with that. – user17732522 Jan 14 '23 at 14:20
  • Instead of writing and calling `pass_back_ownership(std::move(ptr))` why not just call `ptr.reser();`? Or, *better still* just let `ptr` go out of scope? – Galik Jan 14 '23 at 14:29

1 Answers1

1

In the code you have written the task object will be destroyed (with a call to the destructor) at the end of pass_back_ownership.

That is completely fine and no UB in your code. However, main must be careful not to dereference ptr after the call to pass_back_ownership.

Also be careful that there are potential order-of-evaluation issues if pass_back_ownership were to take the parameter by-value instead of by-reference. That would make the code a bit simpler, since you wouldn't need to crate the local tmp variable, but only since C++17 is it guaranteed that ptr->pass_back_ownership will be evaluated before the function parameter is constructed. If the order was the other way around, then you would have UB for dereferencing an empty ptr.

Regarding best practice:

What your call ptr->pass_back_ownership(std::move(ptr)); is saying is that main relinquishes ownership of the task object referenced by ptr to itself. The lifetime of the task object is not intended to be bound to the scope of main anymore.

The question would then be what possible lifetimes the task object could then have. There are basically only two choices: 1. its lifetime ends at latest at the end of the function call, which is your case, or 2. pass_back_ownership stores the passed unique_ptr somewhere outside the function to extend the lifetime.

If you are interested only in 1., then I would question why it is necessary to pass the std::unique_ptr to some random function to achieve the effect. If you intent ptr->pass_back_ownership(std::move(ptr)); to definitively destroy the task object, then why aren't the operations that pass_back_ownership does simply done in the destructor of task? Then you could simply let ptr in main run out-of-scope to have the same effect.

It makes a bit more sense if pass_back_ownership only conditionally actually takes ownership by moving the unique_ptr reference. Then main could test with if(ptr) after the call whether or not the task object was destroyed, but then why is it important for the task object to be destroyed prematurely instead of simply waiting for ptr in main to go out-of-scope? That no instructions are left can also be communicated by either having the function return a bool state or have the task object have some way of checking presence of instructions (e.g. an operator bool).

user17732522
  • 53,019
  • 2
  • 56
  • 105
  • The unique_ptr shall be picked up by the actual task running on top of the task-class stackframe at the end of its lifetime. This means that pass_back_ownership would temporarily store the pointer in its own stackframe and then have it atomically exchanged by the running task at the end of lifetime, thus making sure the task object is deleted with the task. – glades Jan 14 '23 at 14:32
  • @glades Sorry, but I think there is too much context missing here. Are you suggesting that there are multiple threads involved? Everything I have seen in this Q&A so far suggests only a single thread with a single thread with a single stack. I don't get from your description why simpler approaches are not sufficient to guarantee that the `task` will be deleted. – user17732522 Jan 14 '23 at 14:38
  • My mistake, I didn't state it in the description, but the code is for a microcontroller with sophisticated task management and I try to build a wrapper for that task system which is abstracted away in this task-class. Imagine that the task control block uses resources in the stackframe of the task-object and the task is started via constructor- and deleted via destructor calls of the task object. I can't show it here because it doesn't compile on godbolt. – glades Jan 14 '23 at 14:46
  • @glades I would suggest a look at how `std::jthtread`'s interface works. It represents a class that manages the actual thread. It doesn't actually live on the thread's stack or anything like that. The destructor therefore should be called from the thread (task) that _owns_ the thread, not the thread itself. The destructor then takes appropriate action to wait for the thread to finish. The lifetime of the `std::jthread` object doesn't match the lifetime of the thread. Instead it has an empty state which it can hold before or after the thread runs. I think your `task` class may behave similarly. – user17732522 Jan 14 '23 at 14:56
  • @glades Also note that `std::unique_ptr` is not thread-safe and certainly not signal/interrupt-safe. You need at a minimum some form of synchronization if `auto tmp = std::move(ptr);` is running on another thread, in a signal handler or interrupt handler. (The standard doesn't say anything about interrupts, but in a signal handler `std::unique_ptr` isn't guaranteed to work at all according to the standard. Of course the implementation may make extra guarantees.) – user17732522 Jan 14 '23 at 14:59