Lifetime, destructors, and sub-objects

[Markus Moll]

Hi

I am suddenly questioning the correctness of a pattern that I've used for
a while now and would like to gather opinions before I make my code
unnecessarily illegible :-)

Let's say I have the following class (struct for brevity):

struct A : NonTrivialBaseClassWhoseContentsDoNotMatter
{
std::atomic<bool> flag;

~A();
};

and an object 'a' of type A.

Now I have a thread that keeps a pointer to a, 'pa', and occasionally
sets 'pa->flag'. Additionally, there's a function stop_thread() that will
stop that thread. The thread is guaranteed not to access 'a' after a call
to stop_thread.

What I've been doing in the past was to call stop_thread() in A's
destructor to make sure that 'a' will not be accessed after its
destruction:

A::~A()
{
stop_thread();
}

However, a closer reading of the relevant sections in the current C++
draft led me to thinking that this can actually result in undefined
behavior: Per 3.8, an object's lifetime ends when its destructor is
called (if that constructor is non-trivial). Accessing any non-static
members of an object whose lifetime has ended is undefined behavior.

Now oddly enough, there seems to be a very easy but awkward way around
this. The destructors of a's sub-objects will only be called once the
body of A's destructor is left. Therefore, their lifetimes end only then.
So to me, the following appears to have defined behavior:

struct A : NonTrivialBaseClassWhoseContentsDoNotMatter
{
std::atomic<bool> flag;
};

struct B : A
{
~B();
};

B::~B()
{
stop_thread();
}

Does the original code really exhibit undefined behavior? Does adding
another derived class really help?

Any insights are greatly appreciated

Markus

[James Kuyper]

On 12/4/18 08:53, Markus Moll wrote:
> Hi
>
> I am suddenly questioning the correctness of a pattern that I've used for
> a while now and would like to gather opinions before I make my code
> unnecessarily illegible :-)
>
> Let's say I have the following class (struct for brevity):
>
> struct A : NonTrivialBaseClassWhoseContentsDoNotMatter
> {
> std::atomic<bool> flag;
>
> ~A();
> };
>
> and an object 'a' of type A.
>
> Now I have a thread that keeps a pointer to a, 'pa', and occasionally
> sets 'pa->flag'. Additionally, there's a function stop_thread() that will
> stop that thread. The thread is guaranteed not to access 'a' after a call
> to stop_thread.
>
> What I've been doing in the past was to call stop_thread() in A's
> destructor to make sure that 'a' will not be accessed after its
> destruction:
>
> A::~A()
> {
> stop_thread();
> }
>
> However, a closer reading of the relevant sections in the current C++
> draft led me to thinking that this can actually result in undefined
> behavior: Per 3.8, an object's lifetime ends when its destructor is
> called (if that constructor is non-trivial). Accessing any non-static
> members of an object whose lifetime has ended is undefined behavior.

The lifetime of an object of type A ends when ~A() starts. However, the
lifetime of A's member objects only ends when they themselves get
destroyed, which occurs at the end of ~A(). It's perfectly safe to
access them from within ~A(). Note the word "finishes" in the following
clause:

"For an object with a non-trivial destructor, referring to any
non-static member or base class of the object after the destructor
finishes execution results in undefined behavior." (12.7p1).

[Alf P. Steinbach]

On 04.12.2018 14:53, Markus Moll wrote:
> Accessing any non-static
> members of an object whose lifetime has ended is undefined behavior.

No. The formal lifetime ends with start of the destructor execution (if
any). But:

C++17 §15.7/1
[quote]

For an object with a non-trivial destructor, referring to any non-static
member or base class of the object after the destructor finishes
execution results in undefined behavior.

[/quote]

I.e. you're safe. :)

It might feel a little bit weird that during the destructor execution
there is an existing object whose lifetime has ended. But this is
similar to how, during a constructor execution, there is an existing
object whose lifetime has not yet started. It would be difficult to
initialize an object if you couldn't do anything with it in the
constructor body. After the constructor body finishes and the lifetime
starts, the object might turn `const`, say. Similarly, its `const`-ness,
if any, disappears when its lifetime ends and the destructor is invoked.

Maybe think of it as a building.

With a sufficiently simple view the building's lifetime as a proper
building doesn't start until construction is finished (we disregard the
common practice of doing some final things even after one has started
using the building), and doesn't extend into the eventual destruction.


Cheers & hth.,

- Alf

[Markus Moll]

On Tue, 04 Dec 2018 15:14:40 +0100, Alf P. Steinbach wrote:

> On 04.12.2018 14:53, Markus Moll wrote:
>> Accessing any non-static members of an object whose lifetime has ended
>> is undefined behavior.
>
> No. The formal lifetime ends with start of the destructor execution (if
> any). But:
>
> C++17 §15.7/1 [quote]
> For an object with a non-trivial destructor, referring to any non-static
> member or base class of the object after the destructor finishes
> execution results in undefined behavior.
> [/quote]
>
> I.e. you're safe. :)
>

Hm, I agree that accessing the sub-objects is safe (which is what I rely
on in my revised code example). However, the problem seems to be
accessing them through a pointer to A, as 3.8 states:

[quote]
(...) after the lifetime of an object has ended and before the storage
which the object occupied is reused or released, any pointer that refers
to the storage location where the object will be or was located
may be used but only in limited ways. (...) The program has undefined
behavior if:
(...)
- the pointer is used to access a non-static data member or call a non-
static member function of the object
[/quote]

And yes, I think it would be safe to store a pointer to the atomic<bool>
in the thread and use that pointer. However, I would prefer not to do
that. Background: the thread in my example is a timer thread. Users can
add or remove timer callbacks of type "void (*)(void*)". I follow the
familiar pattern of using a static member with that signature which
converts its argument to A* and then calls a corresponding non-static
member function without further arguments. That call, in my opinion,
would be undefined behavior. (And I find that both odd and surprising)

Markus

[Markus Moll]

Oh, alright, I just realized that 3.8 refers to 12.7 for objects under
construction or destruction and that the restriction I quoted really only
applies to "raw storage". That makes a lot more sense :-)

Thanks everyone for your help
Markus