Skopiować konstruktor z argumentem non-const sugerowanym przez reguły bezpieczeństwa wątków?

Mam opakowanie jakiegoś starszego kodu.

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

W tym starszym kodzie funkcja „duplikująca” obiekt nie jest bezpieczna dla wątków (podczas wywoływania tego samego pierwszego argumentu), dlatego nie jest oznaczona constw opakowaniu. Chyba następujące współczesne zasady: https://herbsutter.com/2013/01/01/video-you-dont-know-const-and-mutable/

To duplicatewygląda na dobry sposób na implementację konstruktora kopii, z wyjątkiem szczegółów, które nie są const. Dlatego nie mogę tego zrobić bezpośrednio:

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

Jakie jest zatem wyjście z tej paradoksalnej sytuacji?

(Powiedzmy również, że legacy_duplicatenie jest to bezpieczne dla wątków, ale wiem, że opuszcza obiekt w oryginalnym stanie po wyjściu. Jako funkcja C zachowanie jest tylko udokumentowane, ale nie ma pojęcia stałości).

Mogę wymyślić wiele możliwych scenariuszy:

(1) Jedną z możliwości jest to, że nie ma możliwości zaimplementowania konstruktora kopii o zwykłej semantyce. (Tak, mogę przesunąć obiekt i to nie jest to, czego potrzebuję.)

(2) Z drugiej strony, kopiowanie obiektu z natury nie jest bezpieczne w tym sensie, że kopiowanie prostego typu może znaleźć źródło w stanie częściowo zmodyfikowanym, więc mogę po prostu iść do przodu i zrobić to, być może,

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) lub nawet po prostu deklarować duplicateconst i kłamać na temat bezpieczeństwa wątków we wszystkich kontekstach. (Po tym wszystkim, na czym polega starsza funkcja, constkompilator nawet nie narzeka.)

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

(4) Na koniec mogę postępować zgodnie z logiką i stworzyć konstruktor kopii, który przyjmuje argument nie-stały .

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

Okazuje się, że działa to w wielu kontekstach, ponieważ te obiekty zwykle nie są const.

Pytanie brzmi, czy jest to ważna czy wspólna trasa?

Nie potrafię ich nazwać, ale intuicyjnie oczekuję wielu problemów na drodze do posiadania konstruktora kopii non-const. Prawdopodobnie nie będzie się kwalifikować jako typ wartości z powodu tej subtelności.

(5) Wreszcie, chociaż wydaje się to przesadą i może mieć wysokie koszty w czasie wykonywania, mogę dodać muteks:

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

Ale zmuszenie do zrobienia tego wygląda na pesymalizację i powiększa klasę. Nie jestem pewien. Obecnie skłaniam się w kierunku (4) lub (5) lub kombinacji obu.

EDYCJA 1:

Inna opcja:

(6) Zapomnij o braku zrozumienia funkcji duplikatu elementu i po prostu wywołaj legacy_duplicatez konstruktora i zadeklaruj, że konstruktor kopiowania nie jest bezpieczny dla wątków. (I jeśli to konieczne, wykonaj kolejną bezpieczną dla wątku wersję tego typu A_mt).

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

EDYCJA 2:

Może to być dobry model do tego, co robi starsza funkcja. Zauważ, że dotknięcie wejścia powoduje, że wywołanie nie jest bezpieczne dla wątku w odniesieniu do wartości reprezentowanej przez pierwszy 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; 
}

EDYCJA 3: Niedawno dowiedziałem się, że std::auto_ptrmiał podobny problem z konstruktorem „kopiowania” innym niż const. Efekt był taki, że auto_ptrnie można go użyć w pojemniku. https://www.quantstart.com/articles/STL-Containers-and-Auto_ptrs-Why-They-Dont-Mix/

Chciałbym tylko uwzględnić obie opcje (4) i (5), ale jednoznacznie wyrazić zgodę na zachowanie niebezpieczne dla wątków, jeśli uważasz, że jest to konieczne dla wydajności.

Oto kompletny przykład.

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

Wynik:

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

Jest to zgodne z przewodnikiem po stylu Google, w którym constkomunikuje się bezpieczeństwo wątków, ale kod wywołujący interfejs API może zrezygnowaćconst_cast

TLDR: Albo naprawić realizację swojej funkcji powielanie lub wprowadzić mutex (lub trochę bardziej odpowiednie urządzenie blokujące, być może Spinlock, lub upewnić się, że mutex jest skonfigurowany do korkociągu zanim cokolwiek cięższy) do teraz , a następnie naprawić realizację powielania i usuń blokadę, gdy blokowanie faktycznie stanie się problemem.

Myślę, że kluczową kwestią, na którą należy zwrócić uwagę, jest to, że dodajesz funkcję, która wcześniej nie istniała: możliwość duplikowania obiektu z wielu wątków jednocześnie.

Oczywiście w warunkach, które opisałeś, byłby to błąd - warunek wyścigu, gdybyś to robił wcześniej, bez korzystania z jakiejś zewnętrznej synchronizacji.

Dlatego użycie tej nowej funkcji będzie dodawane do kodu, a nie dziedziczone jako istniejąca funkcjonalność. Powinieneś być tym, który wie, czy dodanie dodatkowego blokowania będzie faktycznie kosztowne - w zależności od tego, jak często będziesz używać tej nowej funkcji.

Ponadto, w oparciu o postrzeganą złożoność obiektu - przez specjalne traktowanie, które mu dajesz, założę, że procedura powielania nie jest trywialna, a zatem już dość droga pod względem wydajności.

W oparciu o powyższe masz dwie ścieżki, którymi możesz podążać:

A) Wiesz, że kopiowanie tego obiektu z wielu wątków nie będzie się odbywać wystarczająco często, aby narzut dodatkowego blokowania był kosztowny - być może banalnie tani, przynajmniej biorąc pod uwagę, że istniejąca procedura kopiowania jest sama w sobie dość kosztowna, jeśli używasz muteks spinlock / pre-spinning i nie ma z tym żadnej niezgody.

B) Podejrzewasz, że kopiowanie z wielu wątków będzie się odbywać wystarczająco często, aby dodatkowe blokowanie stanowiło problem. Zatem naprawdę masz tylko jedną opcję - napraw kod duplikacji. Jeśli tego nie naprawisz, i tak będziesz musiał zablokować, czy to na tej warstwie abstrakcji, czy gdzie indziej, ale będziesz go potrzebował, jeśli nie chcesz błędów - i jak ustaliliśmy, na tej ścieżce zakładasz blokowanie będzie zbyt kosztowne, dlatego jedyną opcją jest naprawienie kodu duplikacji.

Podejrzewam, że naprawdę znajdujesz się w sytuacji A i po prostu dodanie blokady / wirującego muteksu, który nie ma żadnej obniżki wydajności, gdy jest niekwestionowany, będzie działać dobrze (pamiętaj jednak, aby go przetestować).

Teoretycznie istnieje inna sytuacja:

C) W przeciwieństwie do pozornej złożoności funkcji duplikacji, jest ona w rzeczywistości trywialna, ale z jakiegoś powodu nie można jej naprawić; jest tak trywialny, że nawet niekwestionowana blokada wprowadza niedopuszczalne pogorszenie wydajności duplikacji; powielanie wątków równoległych jest rzadko stosowane; duplikacja na jednym wątku jest używana przez cały czas, przez co pogorszenie wydajności jest absolutnie nie do przyjęcia.

W takim przypadku sugeruję, aby: zadeklarować usunięcie domyślnych konstruktorów / operatorów kopiowania, aby zapobiec przypadkowemu ich użyciu. Utwórz dwie jawnie wywoływalne metody powielania: bezpieczną dla wątku i niebezpieczną dla wątku; spraw, aby użytkownicy dzwonili do nich jawnie, w zależności od kontekstu. Ponownie, nie ma innego sposobu na osiągnięcie akceptowalnej wydajności pojedynczego wątku i bezpiecznego wielowątkowości, jeśli naprawdę jesteś w takiej sytuacji i po prostu nie możesz naprawić istniejącej implementacji duplikacji. Ale wydaje mi się, że jest mało prawdopodobne, że tak naprawdę jesteś.

Wystarczy dodać ten mutex / spinlock i test porównawczy.