Copier le constructeur avec un argument non-const suggéré par les règles de sécurité des threads?

J'ai un wrapper pour un morceau de code hérité.

class A{
   L* impl_; // the legacy object has to be in the heap, could be also unique_ptr
   A(A const&) = delete;
   L* duplicate(){L* ret; legacy_duplicate(impl_, &L); return ret;}
   ... // proper resource management here
};

Dans ce code hérité, la fonction qui «duplique» un objet n'est pas thread-safe (lors de l'appel au même premier argument), elle n'est donc pas marquée constdans le wrapper. Je suppose que les règles modernes suivantes: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Cela duplicateressemble à un bon moyen d'implémenter un constructeur de copie, à l'exception du détail qui ne l'est pas const. Je ne peux donc pas le faire directement:

class A{
   L* impl_; // the legacy object has to be in the heap
   A(A const& other) : L{other.duplicate()}{} // error calling a non-const function
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

Alors, comment sortir de cette situation paradoxale?

(Disons aussi que ce legacy_duplicaten'est pas thread-safe mais je sais que l'objet reste dans son état d'origine à sa sortie. Étant une fonction C, le comportement est seulement documenté mais n'a pas de concept de constance.)

Je peux penser à de nombreux scénarios possibles:

(1) Une possibilité est qu'il n'y a aucun moyen d'implémenter un constructeur de copie avec la sémantique habituelle. (Oui, je peux déplacer l'objet et ce n'est pas ce dont j'ai besoin.)

(2) D'un autre côté, la copie d'un objet est intrinsèquement non thread-safe dans le sens où la copie d'un type simple peut trouver la source dans un état semi-modifié, donc je peux simplement avancer et le faire peut-être,

class A{
   L* impl_;
   A(A const& other) : L{const_cast<A&>(other).duplicate()}{} // error calling a non-const function
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

(3) ou même simplement déclarer duplicateconst et mentir sur la sécurité des threads dans tous les contextes. (Après tout, la fonction héritée ne se soucie pas, constdonc le compilateur ne se plaindra même pas.)

class A{
   L* impl_;
   A(A const& other) : L{other.duplicate()}{}
   L* duplicate() const{L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

(4) Enfin, je peux suivre la logique et créer un constructeur de copie qui prend un argument non const .

class A{
   L* impl_;
   A(A const&) = delete;
   A(A& other) : L{other.duplicate()}{}
   L* duplicate(){L* ret; legacy_duplicate(impl_, &ret); return ret;}
};

Il s'avère que cela fonctionne dans de nombreux contextes, car ces objets ne le sont généralement pas const.

La question est, est-ce un itinéraire valable ou commun?

Je ne peux pas les nommer, mais je m'attends intuitivement à beaucoup de problèmes sur la route d'avoir un constructeur de copie non-const. Il ne sera probablement pas considéré comme un type de valeur en raison de cette subtilité.

(5) Enfin, bien que cela semble être une exagération et pourrait avoir un coût d'exécution élevé, je pourrais ajouter un mutex:

class A{
   L* impl_;
   A(A const& other) : L{other.duplicate_locked()}{}
   L* duplicate(){
      L* ret; legacy_duplicate(impl_, &ret); return ret;
   }
   L* duplicate_locked() const{
      std::lock_guard<std::mutex> lk(mut);
      L* ret; legacy_duplicate(impl_, &ret); return ret;
   }
   mutable std::mutex mut;
};

Mais être forcé de faire cela ressemble à de la pessimisation et agrandit la classe. Je ne suis pas sûr. Je penche actuellement vers (4) ou (5) ou une combinaison des deux.

—— MODIFIER

Une autre option:

(6) Oubliez tout le non-sens de la fonction membre en double et appelez simplement à legacy_duplicatepartir du constructeur et déclarez que le constructeur de copie n'est pas thread-safe. (Et si nécessaire, faites une autre version thread-safe du type, A_mt)

class A{
   L* impl_;
   A(A const& other){legacy_duplicate(other.impl_, &impl_);}
};

EDIT 2

Cela pourrait être un bon modèle pour ce que fait la fonction héritée. Notez qu'en touchant l'entrée, l'appel n'est pas thread-safe par rapport à la valeur représentée par le premier argument.

void legacy_duplicate(L* in, L** out){
   *out = new L{};
   char tmp = in[0];
   in[0] = tmp; 
   std::memcpy(*out, in, sizeof *in); return; 
}

J'inclurais simplement vos options (4) et (5), mais vous vous engagez explicitement à adopter un comportement non thread-safe lorsque vous pensez que cela est nécessaire pour les performances.

Voici un exemple complet.

#include <cstdlib>
#include <thread>

struct L {
  int val;
};

void legacy_duplicate(const L* in, L** out) {
  *out = new L{};
  std::memcpy(*out, in, sizeof *in);
  return;
}

class A {
 public:
  A(L* l) : impl_{l} {}
  A(A const& other) : impl_{other.duplicate_locked()} {}

  A copy_unsafe_for_multithreading() { return {duplicate()}; }

  L* impl_;

  L* duplicate() {
    printf("in duplicate\n");
    L* ret;
    legacy_duplicate(impl_, &ret);
    return ret;
  }
  L* duplicate_locked() const {
    std::lock_guard<std::mutex> lk(mut);
    printf("in duplicate_locked\n");
    L* ret;
    legacy_duplicate(impl_, &ret);
    return ret;
  }
  mutable std::mutex mut;
};

int main() {
  A a(new L{1});
  const A b(new L{2});

  A c = a;
  A d = b;

  A e = a.copy_unsafe_for_multithreading();
  A f = const_cast<A&>(b).copy_unsafe_for_multithreading();

  printf("\npointers:\na=%p\nb=%p\nc=%p\nc=%p\nd=%p\nf=%p\n\n", a.impl_,
     b.impl_, c.impl_, d.impl_, e.impl_, f.impl_);

  printf("vals:\na=%d\nb=%d\nc=%d\nc=%d\nd=%d\nf=%d\n", a.impl_->val,
     b.impl_->val, c.impl_->val, d.impl_->val, e.impl_->val, f.impl_->val);
}

Production:

in duplicate_locked
in duplicate_locked
in duplicate
in duplicate

pointers:
a=0x7f85e8c01840
b=0x7f85e8c01850
c=0x7f85e8c01860
c=0x7f85e8c01870
d=0x7f85e8c01880
f=0x7f85e8c01890

vals:
a=1
b=2
c=1
c=2
d=1
f=2

Cela suit le guide de style Google dans lequel constcommunique la sécurité des threads, mais le code appelant votre API peut se désinscrire en utilisantconst_cast

TLDR: Soit corriger l'implémentation de votre fonction de duplication, soit introduire un mutex (ou un dispositif de verrouillage plus approprié, peut-être un spinlock, ou assurez-vous que votre mutex est configuré pour tourner avant de faire quoi que ce soit de plus lourd) pour l'instant , puis corrigez l'implémentation de la duplication et retirez le verrouillage lorsque le verrouillage devient réellement un problème.

Je pense qu'un point clé à noter est que vous ajoutez une fonctionnalité qui n'existait pas auparavant: la possibilité de dupliquer un objet à partir de plusieurs threads en même temps.

Évidemment, dans les conditions que vous avez décrites, cela aurait été un bug - une condition de concurrence, si vous l'aviez fait auparavant, sans utiliser une sorte de synchronisation externe.

Par conséquent, toute utilisation de cette nouvelle fonctionnalité sera quelque chose que vous ajouterez à votre code, et non pas héritée en tant que fonctionnalité existante. Vous devriez être celui qui sait si l'ajout du verrouillage supplémentaire sera réellement coûteux - selon la fréquence à laquelle vous allez utiliser cette nouvelle fonctionnalité.

En outre, en fonction de la complexité perçue de l'objet - par le traitement spécial que vous lui donnez, je vais supposer que la procédure de duplication n'est pas triviale, donc, déjà assez coûteuse en termes de performances.

Sur la base de ce qui précède, vous avez deux voies que vous pouvez suivre:

A) Vous savez que la copie de cet objet à partir de plusieurs threads ne se produira pas assez souvent pour que la surcharge du verrouillage supplémentaire soit coûteuse - peut-être trivialement bon marché, du moins étant donné que la procédure de duplication existante est assez coûteuse en soi, si vous utilisez un spinlock / pré-rotation mutex, et il n'y a aucune contestation à ce sujet.

B) Vous pensez que la copie à partir de plusieurs threads se produira assez souvent pour que le verrouillage supplémentaire soit un problème. Ensuite, vous n'avez vraiment qu'une seule option: corriger votre code de duplication. Si vous ne le corrigez pas, vous devrez de toute façon verrouiller, que ce soit sur cette couche d'abstraction ou ailleurs, mais vous en aurez besoin si vous ne voulez pas de bugs - et comme nous l'avons établi, dans ce chemin, vous supposez ce verrouillage sera trop coûteux, par conséquent, la seule option consiste à corriger le code de duplication.

Je soupçonne que vous êtes vraiment dans la situation A, et l'ajout d'un mutex spinlock / spinning qui n'a pratiquement aucune pénalité de performance lorsqu'il n'est pas contesté, fonctionnera très bien (n'oubliez pas de le comparer, cependant).

Il y a, en théorie, une autre situation:

C) Contrairement à la complexité apparente de la fonction de duplication, elle est en fait triviale, mais ne peut pas être corrigée pour une raison quelconque; il est si trivial que même un verrou tournant non contesté introduit une dégradation des performances inacceptable à la duplication; la duplication sur les threads parallèles est rarement utilisée; la duplication sur un seul thread est utilisée tout le temps, ce qui rend la dégradation des performances absolument inacceptable.

Dans ce cas, je suggère ce qui suit: déclarer les constructeurs / opérateurs de copie par défaut supprimés, pour empêcher quiconque de les utiliser accidentellement. Créez deux méthodes de duplication explicitement appelables, une méthode thread-safe et une thread thread unsafe; incitez vos utilisateurs à les appeler explicitement, selon le contexte. Encore une fois, il n'y a aucun autre moyen d'obtenir des performances acceptables pour un seul thread et un multithread sûr, si vous êtes vraiment dans cette situation et que vous ne pouvez tout simplement pas corriger l'implémentation de duplication existante. Mais je pense qu'il est très peu probable que vous le soyez vraiment.

Ajoutez simplement ce mutex / spinlock et référence.