Default constructor of std::atomic_flag

[Thibault Lescoat]

Hello,

For the moment initializing a std::atomic_flag is only really supported if
you use ATOMIC_FLAG_INIT (else the state is undetermined, which is useless
and error-prone). Part of the reason why is explained here:
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1379.htm. Which boils down
to: it is this way because of "compatibility" with C. Another reason is that
the clear state is not necessarily 0 on some platforms. Also,
std::atomic_flag do have a default constructor.

What I see as a defect is that this default constructor is not equivalent to
"= ATOMIC_FLAG_INIT". For me, the 2 following lines should be strictly
equivalent:

std::atomic_flag a;
std::atomic_flag b = ATOMIC_FLAG_INIT;

The fact that some platforms store "clear" as 1 is not incompatible with
having a more intelligent default-constructor.

Also, has someone already reported it ?

[Thiago Macieira]

Em segunda-feira, 7 de março de 2016, às 20:20:23 PST, Thibault Lescoat
escreveu:

> For the moment initializing a std::atomic_flag is only really supported if
> you use ATOMIC_FLAG_INIT (else the state is undetermined, which is useless
> and error-prone). Part of the reason why is explained here:
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1379.htm. Which boils down
> to: it is this way because of "compatibility" with C. Another reason is that
> the clear state is not necessarily 0 on some platforms. Also,
> std::atomic_flag do have a default constructor.
>
> What I see as a defect is that this default constructor is not equivalent to
> "= ATOMIC_FLAG_INIT". For me, the 2 following lines should be strictly
> equivalent:
>
> std::atomic_flag a;
> std::atomic_flag b = ATOMIC_FLAG_INIT;

Why should they be the same, if they are not the same in C?

--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center

[Andrey Semashev]

On 2016-03-07 22:20, Thibault Lescoat wrote:
> Hello,
>
> For the moment initializing a std::atomic_flag is only really supported if
> you use ATOMIC_FLAG_INIT (else the state is undetermined, which is useless
> and error-prone).

It's not useless. See the recent thread:

LWG1146 / N2992: static constexpr bool atomic::is_lock_free()

[Andrey Semashev]

On 2016-03-09 03:12, thibault...@telecom-paristech.org wrote:
>
> @Andrey:
> I'm sorry, I failed to see the link with atomic_flag ? (or at least the
> thread you mention). What I think is that it is a defect
>
> @all:
> From the standard (N4296), §29.7.3: The atomic_flag type shall have
> standard layout. It shall have a trivial default constructor, a deleted
> copy constructor, a deleted copy assignment operator, and a trivial
> destructor.
> For me this is the defect line. Is there a case where a trivial default
> constructor is indispensable ?

That thread I mentioned discusses the use of atomic<> in process-shared
memory. The fact that the default constructor does not initialize the
value is crucial in that context. atomic_flag has the same property.

[Nevin Liber]

On 11 March 2016 at 15:05, Andrey Semashev <andrey....@gmail.com> wrote:

That thread I mentioned discusses the use of atomic<> in process-shared memory. The fact that the default constructor does not initialize the value is crucial in that context.

Is it crucial that the default constructor not initialize the value or is it just that there needs to be a way to construct it w/o initializing the value (something like atomic::atomic(default_initialized_t), for instance)?

The default constructor of atomic not initializing the value is fairly error-prone.

--

 Nevin ":-)" Liber  <mailto:ne...@eviloverlord.com>  +1-847-691-1404

[Thiago Macieira]

On terça-feira, 8 de março de 2016 16:12:48 PST thibault.lescoat@telecom-
paristech.org wrote:
> Well, because it is that way in C does not mean C++ have to retain it - or
> else lot of parts of C++ would simply not exist !
> Here I think that the default constructor should simply initialize it with
> a determined state (clear) - note that I do not say a determined value,
> which is obviously implementation defined.

Usually, the exact same code (identical!) behaves the same way in C and C++,
provided it compiles.

[Andrey Semashev]

On 2016-03-12 00:10, Nevin Liber wrote:
> On 11 March 2016 at 15:05, Andrey Semashev <andrey....@gmail.com

> <mailto:andrey....@gmail.com>> wrote:
>
>
> That thread I mentioned discusses the use of atomic<> in
> process-shared memory. The fact that the default constructor does
> not initialize the value is crucial in that context.
>
>
> Is it crucial that the default constructor not initialize the value or
> is it just that there needs to be a way to construct it w/o initializing
> the value (something like atomic::atomic(default_initialized_t), for
> instance)?

It is crucial because if not, there is no way to make the constructor
trivial. Even if atomic::atomic(default_initialized_t) does not
initialize the value, this is not a trivial constructor and as such it
requires the constructor to be called before using the atomic value.

The simplified typical code for shared memory initialization goes along
these lines:

struct shm_header
{
enum init_state : uint32_t
{
uninitialized = 0,
in_progress = 1,
initialized = 2
};
atomic<init_state> m_initialized;
my_data m_my_data;

shm_header() : m_my_data(non_trivial_initialization_here)
{
m_initialized.store(initialized, release);
}
};

shm_header* hdr = allocate_or_open_shm();

shm_header::init_state old_state = hdr->m_initialized.load(acquire);
while (old_state != shm_header::initialized)
{
if (old_state == shm_header::uninitialized &&
hdr->m_initialized.compare_exchange_weak(old_state,
shm_header::in_progress, acq_rel, acquire))
{
new (hdr) shm_header();
break;
}

// wait for initialization to complete

old_state = hdr->m_initialized.load(acquire);
}

[thibault...@telecom-paristech.org]

IMHO, your example is broken, because on this line:

    shm_header* hdr  = allocate_or_open_shm();

For the first call of this function, the returned hdr->m_initialized must be initialized to shm_header::uninitialized - or else, the initialization won't be done (worse, if it's first value is shm_header::in_progress, then it will just dead lock all threads trying to get the resource). So, default constructor or no default constructor, an initialization of this atomic is still needed. Note that your example also isn't 100% on the point, as it does not use atomic_flag.
This is similar for atomic_flag: you just can't rely on it until it is properly initialized, either with ATOMIC_FLAG_INIT, or with (way better) a real default constructor that does it. I have seen such errors a number of times, where people were assuming a default constructed atomic_flag was cleared...

[Andrey Semashev]

On 2016-03-13 14:50, thibault...@telecom-paristech.org wrote:
> IMHO, your example is broken, because on this line:
>
> shm_header* hdr = allocate_or_open_shm();
>
> For the first call of this function, the returned hdr->m_initialized

> *must *be initialized to shm_header::uninitialized

Shared memory segments are zero initialized by OS upon creation. That is
guaranteed by allocate_or_open_shm(), if you like.

> Note that your example
> also isn't 100% on the point, as it does not use atomic_flag.

I could have used atomic_flag to implement a spin lock instead of an
atomic state variable.

> This is similar for atomic_flag: you just can't rely on it until it is
> properly initialized, either with ATOMIC_FLAG_INIT, or with (way better)
> a real default constructor that does it. I have seen such errors a
> number of times, where people were assuming a default constructed
> atomic_flag was cleared...

Well, atomics are hard. Missing initialization is the least of your
concerns.

If you do want non-trivial default constrictor, then create a wrapper
class that derives from atomic_flag or atomic, which calls the
initializing constructor in its own constructor. Whether such wrapper
should be part of the standard library or not I won't argue. My point is
that the triviality of the default constructor of std::atomic and
std::atomic_flag is an important feature and it has to stay that way.

> Le vendredi 11 mars 2016 22:48:31 UTC+1, Andrey Semashev a écrit :
>
> On 2016-03-12 00:10, Nevin Liber wrote:
> > On 11 March 2016 at 15:05, Andrey Semashev <andrey....@gmail.com

> <javascript:>

[thibault...@telecom-paristech.org]

Shared memory segments are zero initialized by OS upon creation. That is guaranteed by allocate_or_open_shm(), if you like.

Well, at least on Windows it is not true - you have to zero the memory yourself. What's more, the clear state for an atomic_flag is not always 0, on some platforms a cleared atomic_flag. Which means your function still has to initialize the flag - why force it to be explicit, while it fit perfectly the semantics of the default constructor ?

The idea is not to remove ATOMIC_FLAG_INIT. It is that you can omit it because the default constructor will act the same. If you have a case where no constructor will be called, then you have to initialize it explicitely - but for most cases the constructor can be called. The following spinlock, for example, won't work correctly, because it lacks ATOMIC_FLAG_INIT. Adding it is not intuitive, as it is the only place of the standard library where an explicit initialization is required:

struct spinlock
{
    void lock()   { while(flag.test_and_set(std::memory_order_acquire)); }
    void unlock() { flag.clear(std::memory_order_release); }
private:
    std::atomic_flag flag;
};

My point is that the triviality of the default constructor of std::atomic and std::atomic_flag is an important feature and it has to stay that way.

The point of your example is not to call or not a constructor. It is that technically you're using an object before it is constructed (shm_header), and you have to insure that when it is constructed it does not default-initialize the flag to "uninitialized" to avoid multiple initialization. Which, in the case of atomic_flag, can get solved by adding a constructor for a specific value.

[Andrey Semashev]

On 2016-03-13 18:29, thibault...@telecom-paristech.org wrote:
> Shared memory segments are zero initialized by OS upon creation.
> That is guaranteed by allocate_or_open_shm(), if you like.
>
> Well, at least on Windows it is not true - you have to zero the memory
> yourself.

You don't:

The initial contents of the pages in a file mapping object backed by
the operating system paging file are 0 (zero).

https://msdn.microsoft.com/en-us/library/windows/desktop/aa366537%28v=vs.85%29.aspx

And I'm sure any operating system gives similar guarantees. Otherwise
this would be a huge security hole.

> What's more, the clear state for an atomic_flag is not always
> 0, on some platforms a cleared atomic_flag.

Formally, that is true. But I'm yet to see the architecture where the
cleared atomic_flag has non-zero representation.

> Which means your function
> still has to initialize the flag - why force it to be explicit, while it
> fit perfectly the semantics of the default constructor ?

On the architecture with non-zero cleared atomic_flag the atomic_flag
would be unusable in the context I described, unless the OS luckily
initializes the shared memory with the same value as the cleared
atomic_flag. Having non-trivial constructor won't help in this case.

> The idea is not to remove ATOMIC_FLAG_INIT. It is that you can omit it
> because the default constructor will act the same. If you have a case
> where no constructor will be called, then you have to initialize it
> explicitely - but for most cases the constructor can be called.

ATOMIC_FLAG_INIT does not add any value if the default constructor
initializes the flag already. There's no point in having both.

I'm not sure what particular use cases were considered when the
committee standardized ATOMIC_FLAG_INIT without saying that it's
actually a fancy way of zero initialization. I mean, I understand that
not requiring that the cleared state is zero potentially makes the
language more portable, but in reality this just limits the usefullness
of atomic_flag, if you are a language purist. Perhaps there are other
cases when the trivial default constructor is essential and
ATOMIC_FLAG_INIT representation is not that I'm missing.

> The
> following spinlock, for example, won't work correctly, because it lacks
> ATOMIC_FLAG_INIT. Adding it is not intuitive, as it is the only place of
> the standard library where an explicit initialization is required:

By far, it is not the only place.

I don't think I see the problem. The one who writes the spinlock class
will just write it the right way. If he doesn't, he probably shouldn't
be writing it in the first place and use an existing implementation
(gladly, there are plenty of them).

> My point is that the triviality of the default constructor of
> std::atomic and std::atomic_flag is an important feature and it has
> to stay that way.
>
> The point of your example is not to call or not a constructor. It is
> that technically you're using an object before it is constructed
> (shm_header), and you have to insure that when it is constructed it does
> not default-initialize the flag to "uninitialized" to avoid multiple
> initialization. Which, in the case of atomic_flag, can get solved by
> adding a constructor for a specific value.

Again, if atomic_flag constructor is not trivial (e.g. because it
initialized the storage), then atomic_flag is not usable in the context
I presented. The change you're proposing solves the problem of missing
initialization (a developer's error) while it prohibits the use of
atomic_flag in shared memory to control initialization. IMHO, that
exchange is not worthwhile.

[Thiago Macieira]

On domingo, 13 de março de 2016 21:19:10 PDT Andrey Semashev wrote:
> And I'm sure any operating system gives similar guarantees. Otherwise
> this would be a huge security hole.

Technically, all it needs to do is make sure that the previous content isn't
visible. It doesn't have to be zero-filled. The page could be filled with any
other pattern (like 0xcdcdcdcd) or even random data to help catch bugs.

Zero is just more convenient because it maps directly to the zero-
initialisation phase of C and C++ static variables and CPUs have very efficient
ways to zero-fill pages.

[Andrey Semashev]

On 2016-03-13 21:24, Thiago Macieira wrote:
> On domingo, 13 de março de 2016 21:19:10 PDT Andrey Semashev wrote:
>> And I'm sure any operating system gives similar guarantees. Otherwise
>> this would be a huge security hole.
>
> Technically, all it needs to do is make sure that the previous content isn't
> visible. It doesn't have to be zero-filled. The page could be filled with any
> other pattern (like 0xcdcdcdcd) or even random data to help catch bugs.
>
> Zero is just more convenient because it maps directly to the zero-
> initialisation phase of C and C++ static variables and CPUs have very efficient
> ways to zero-fill pages.

Right.