Copiar construtor com argumento não const sugerido por regras de segurança de thread?

Eu tenho um wrapper para algum pedaço de código legado.

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
};

Nesse código legado, a função que "duplica" um objeto não é segura para threads (ao chamar o mesmo primeiro argumento), portanto, não está marcada const no wrapper. Eu acho que seguindo as regras modernas: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

Esta duplicateparece ser uma boa maneira de implementar um construtor de cópia, exceto para o detalhe que não é const. Portanto, não posso fazer isso diretamente:

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;}
};

Então, qual é a saída dessa situação paradoxal?

(Digamos também que legacy_duplicatenão é seguro para threads, mas eu sei que deixa o objeto no estado original quando ele sai. Sendo uma função C, o comportamento é apenas documentado, mas não tem conceito de constância.)

Eu posso pensar em muitos cenários possíveis:

(1) Uma possibilidade é que não há como implementar um construtor de cópias com a semântica usual. (Sim, eu posso mover o objeto e não é disso que preciso.)

(2) Por outro lado, copiar um objeto é inerentemente não seguro para threads, no sentido de que copiar um tipo simples pode encontrar a fonte em um estado semi-modificado, para que eu possa ir adiante e fazer isso talvez,

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 até mesmo declarar duplicateconst e mentir sobre segurança de threads em todos os contextos. (Afinal, a função legada não se importa, constentão o compilador nem se queixará.)

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

(4) Finalmente, posso seguir a lógica e criar um construtor de cópias que usa um argumento não-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;}
};

Acontece que isso funciona em muitos contextos, porque esses objetos geralmente não são const .

A questão é: esta é uma rota válida ou comum?

Não posso nomeá-los, mas, intuitivamente, espero muitos problemas no caminho de ter um construtor de cópias não-const. Provavelmente não se qualificará como um tipo de valor por causa dessa sutileza.

(5) Finalmente, embora isso pareça um exagero e possa ter um alto custo de tempo de execução, eu poderia adicionar um 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;
};

Mas ser forçado a fazer isso parece pessimização e aumenta a classe. Não tenho certeza. Atualmente, estou inclinado a (4) ou (5) ou a uma combinação de ambos.

—— EDITAR

Outra opção:

(6) Esqueça toda a falta de sentido da função de membro duplicado e simplesmente chame legacy_duplicateo construtor e declare que o construtor de cópia não é seguro para threads. (E, se necessário, faça outra versão com segurança para threads do tipo A_mt)

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

EDIT 2

Este poderia ser um bom modelo para o que a função legada faz. Observe que, ao tocar na entrada, a chamada não é segura com relação ao valor representado pelo primeiro argumento.

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

Eu incluiria apenas as suas opções (4) e (5), mas explicitamente optarei pelo comportamento não seguro do thread quando você achar necessário para o desempenho.

Aqui está um exemplo completo.

#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);
}

Resultado:

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

Isso segue o guia de estilo do Google no qual se constcomunica a segurança do encadeamento, mas o código que chama sua API pode desativar usandoconst_cast

TLDR: corrija a implementação da sua função de duplicação ou introduza um mutex (ou algum dispositivo de bloqueio mais apropriado, talvez um spinlock) ou verifique se o mutex está configurado para girar antes de fazer algo mais pesado por enquanto , em seguida, corrija a implementação da duplicação e remova o bloqueio quando o bloqueio realmente se tornar um problema.

Acho que um ponto importante a ser observado é que você está adicionando um recurso que não existia antes: a capacidade de duplicar um objeto de vários threads ao mesmo tempo.

Obviamente, nas condições que você descreveu, isso seria um bug - uma condição de corrida, se você estivesse fazendo isso antes, sem usar algum tipo de sincronização externa.

Portanto, qualquer uso desse novo recurso será algo que você adicionará ao seu código, não herdará como funcionalidade existente. Você deve saber quem adicionar o bloqueio extra será realmente caro - dependendo da frequência com que você usará esse novo recurso.

Além disso, com base na complexidade percebida do objeto - pelo tratamento especial que você está dando a ele, vou assumir que o procedimento de duplicação não é trivial, portanto, já é bastante caro em termos de desempenho.

Com base no exposto, você tem dois caminhos a seguir:

A) Você sabe que copiar este objeto de vários encadeamentos não acontecerá com frequência suficiente para que a sobrecarga do bloqueio adicional seja dispendiosa - provavelmente trivialmente barata, pelo menos uma vez que o procedimento de duplicação existente é dispendioso por si só, se você usar um mutex spinlock / pré-spinning, e não há contenção nele.

B) Você suspeita que a cópia de vários encadeamentos acontecerá com frequência suficiente para que o bloqueio extra seja um problema. Então você realmente tem apenas uma opção - corrija seu código de duplicação. Se você não corrigi-lo, precisará bloquear de qualquer maneira, seja nesta camada de abstração ou em outro lugar, mas será necessário se não desejar erros - e, como estabelecemos, nesse caminho, você assume esse bloqueio será muito caro, portanto, a única opção é corrigir o código de duplicação.

Eu suspeito que você esteja realmente na situação A, e apenas adicionar um mutex spinlock / spinning que tenha quase nenhuma penalidade de desempenho quando não contestado funcionará muito bem (lembre-se de compará-lo).

Existe, em teoria, outra situação:

C) Em contraste com a aparente complexidade da função de duplicação, ela é realmente trivial, mas não pode ser corrigida por algum motivo; é tão trivial que mesmo um spinlock não contestado introduz uma degradação inaceitável do desempenho na duplicação; a duplicação em threads paralelos é usada raramente; a duplicação em um único encadeamento é usada o tempo todo, tornando a degradação do desempenho absolutamente inaceitável.

Nesse caso, sugiro o seguinte: declare os construtores / operadores de cópia padrão excluídos, para impedir que alguém os use acidentalmente. Crie dois métodos de duplicação explicitamente solicitáveis, um segmento seguro e outro não seguro; faça seus usuários chamá-los explicitamente, dependendo do contexto. Novamente, não há outra maneira de obter desempenho aceitável de encadeamento único e multiencadeamento seguro, se você realmente estiver nessa situação e simplesmente não conseguir consertar a implementação de duplicação existente. Mas eu sinto que é altamente improvável que você realmente seja.

Basta adicionar esse mutex / spinlock e referência.