Thread safe move semantics

[German M. Bravo]

When using move semantics (using std::move), not all objects are really moved. For example, Document and Database; instead use intrusive_ptr_nonnull's move constructor which essentially just increments the reference counter and copies the pointer px. This doesn't cut it when one needs to move objects from one thread to other; reason is this produces a race condition in the reference counter, in which both threads could be modifying the non-atomic counter simultaneously.

What I did instead (in Xapiand) is I'm using a so called internal_intrusive_ptr which ensures an object with an "internal" type still remains valid (and with a valid, non-null px) after moving away it's original content. Implementation uses a tmp, from which it extracts the newly created "empty" internal px and can be seen here: ​https://github.com/Kronuz/Xapiand/commit/33ccddbdaa1e3a24a68700af24cf3c7562f43621#diff-bedbbf7159a3d537faadbb10befc15ceR375

This internal_intrusive_ptr could be used everywhere where intrusive_ptr_nonnull is used if the object in question has a default constructor, such as:

• Database
• Document
• ESet
• MSet
• QueryParser
• Registry
• TermGenerator

[Olly Betts]

Was there a reason to set the internal to 0 in the two cases there, rather than just swap the internals? If we swap, the temporary object would unref the swapped in internal, rather than that happening in the assignment.

I'm not sure I see the benefits of internal_intrusive_ptr over just going back to using intrusive_ptr if we can no longer actually enforce the nonnull-ness.

If we could actually keep the nonnull-ness aspect, then I guess the key question is whether the cost of having to allocate a dummy internal for this case (and having to call into the shared library to do it) outweighs the benefits of being able to rely on the internals not being NULL.

[Olly Betts]

I've pushed d730bbfc0773afec3a8f68f7773199aa81376ba8 to master, which addresses this in a simpler way.

The approach I've taken there is simply to relax the non-NULL requirement for the special case of a moved-from intrusive_ptr_nonnull. You can't do much with such a moved-from pointer, but it would be unnatural to try to anyway.

Please let me know if there are any issues with this approach for you.

This code isn't present in 1.4.x, so nothing to consider backporting (which is fortunate, as I suspect we couldn't safely).

[Olly Betts]

Milestone renamed