Ist die Konstruktion eines shared_ptrs aus einem weak_ptr thread safe?

DocShoe

Hallo nochmal,

hab heute nen Lauf

Ich habe folgendes Konstrukt:

#include <chrono>
#include <thread>
#include <memory>

struct Data
{
};

struct Context
{
   std::shared_ptr<Data> Data = std::make_shared<Data>();
};

void thread_func( Cotext& data )
{
   std::weak_ptr wp = data.Data;
   for( ;; )
   {
      std::shared_ptr<Data> sp = wp.lock();
      if( !sp )
      {
         break;
      }
      // do_stuff
   }
}

int main()
{
   Context ctx;
   std::thread t( &thread_func, std::ref( ctx ) );

   std::this_thread::sleep_for(std::chrono::seconds( 5 ) );
   ctx.Data.reset();
}

Ist der Zugriff auf den Memory Control Block des shared_ptr threadsafe? Kann es zu race conditions bei lock() und reset() kommen oder bin ich da sicher?

Quiche Lorraine

@DocShoe
Genau sagen kann ich es dir nicht, aber ich vermute dass es geht.

Ich habe auch mal ein wenig herumgespielt (viele Threads, Timing) und habe es nicht zu einem Fehler bezüglich den Ptr Klassen gebracht. Wohl halt zu den üblichen Sychronisationsproblemen...

Ich würde das Konstrukt aber noch einen Belastungstest mit X Threads und zufälligem Tminig unterziehen.

Finnegan

@DocShoe Erstmal: ist tats tatsächlicher Code? Sollte es nicht void thread_func( Context& data ) lauten, damit das funktioniert?

Zu std::shared_ptr heißt es:

All member functions (including copy constructor and copy assignment) can be called by multiple threads on different shared_ptr objects without additional synchronization even if these objects are copies and share ownership of the same object. If multiple threads of execution access the same shared_ptr object without synchronization and any of those accesses uses a non-const member function of shared_ptr then a data race will occur; the std::atomic<shared_ptr> can be used to prevent the data race.

Zugriff aus verschiedenen Threads ohne Synchronisation auf das shared_ptr-Objekt in Context::Data sehe ich bei dir in Zeile 16 (Initialisierung des weak_ptr) und Zeile 34 (reset()). Letzteres ist eine "non-const member function", daher ist da nach meiner Interpretation des obigen Text ein Data Race. Hier würde ich tatsächlich lieber std::atomic<shared_ptr> verwenden.

Zeile 19 mit dem lock() ist meines Erachtens hingegen unproblematisch. Die Doku sagt zu lock():
Effectively returns expired() ? shared_ptr<T>() : shared_ptr<T>(*this), executed atomically.

Und es handelt sich auch um eigene, thread-lokale Instanzen von weak_ptr und shared_ptr. Der Kontrollblock ist meines Wissens threadsicher, aber wenn man aus verschiedenen Threads auf die selbe shared_ptr-Instanz zugreift, muss man vorsichtig sein. Beim Zugriff auf das von den Pointern gehaltene Objekt (Data) sowieso.

DocShoe

Nein, das ist nur ein Minimalbeispiel. Die echte Anwendung ist ein Datenbank-Zugriffsobjekt, das die echte db-Verbindung intern als shared_ptr hält. Das Datenbank-Zugriffsobjekt bietet eine Funktion an, mit der man einen Listener erzeugt, der wiederum das interne Connection Objekt braucht, um aktiv Nachrichten zu abzuholen. Der Listener bekommt also intern ebenfalls einen shared_ptr auf die db-Verbindung. Daraus ergeben sich jetzt zwei Probleme:

das aktive Abholen der Nachrichten ist unschön, die meiste Zeit werden keine Nachrichten anstehen. Lieber wäre mir da ein nachrichten-basierte Methode, die den Client über einen Callback benachrichtigt.
wenn das Datenbank-Zugriffsobjekt und der Listener (nennen wir ihn einfach mal NotificationReceiver, dann wisst ihr vllt., von welchem DBMS wir hier reden;)) unterschiedliche Lebensdauern haben, und der Listener länger lebt, als das Zugriffsobjekt, denn hält der Listener das interne Verbindungsobjekt durch den shared_ptrlänger am Leben, als es sein muss.

Das hat mich jetzt zu einem asynchronen Ansatz geführt:
Es wird ein eigener Thread gestartet, der aktiv auf Nachrichten prüft und für jede Nachricht einen Callback aufruft. Damit muss der Client selbst nicht mehr aktiv auf Nachrichten prüfen.
Zum zweiten bekommt der Listener keinen shared_ptr mehr, sondern einen weak_ptr, damit kann er erkennen, dass die db-Verbindung getrennt wurde. Wenn lock() keinen gültigen shared_ptr zurückgibt, dann ist die Verbindung getrennt worden und der Thread kann ebenfalls beendet werden. Ich könnte den weak_ptr natürlich schon vorher in einer Umgebung konstruieren, in der garantiert keine race condition auftritt, und ihn dann als Parameter der Thread-Funktion übergeben.

Edit:
std::atomic<shared_ptr<>>hab ich nicht Unser Compiler unterstützt nur C++17.

Edit:
Der zweite Thread zieht natürlich noch andere Probleme nach sich, das war ein guter Hinweis. Vermutlich braucht der Thread eine eigene, exklusive, db Verbindung. Den Zugriff auf die Originalverbindung braucht er weiterhin, um ggf. seine eigene Verbindung abzubauen.

Danke für eure Hinweise.

Finnegan

@DocShoe Nur so ne Idee: Du könntest den weak_ptr evtl im main Thread erstellen und in thread_func eine Kopie entgegennehmen:

void thread_func(std::weak_ptr<Data> wp)
{
   for( ;; )
   {
      std::shared_ptr<Data> sp = wp.lock();
      if( !sp )
      {
         break;
      }
      // do_stuff
   }
}

...
std::thread t( &thread_func, ctx.Data );

Das sollte eigentlich funktionieren so wie ich die Regeln verstehe. Hier hat der Thread seine eigene Instanz des weak_ptr und es findet kein konkurrierendes Lesen/Schreiben des selben Objekts statt - bis eben auf den Kontrollblock, aber der ist ja synchronisiert. Oder hab ich hier einen Denkfehler?

Hier wird der weak_ptr im main thread erstellt und an std::thread(...) übergeben (andere Instanz), dann macht davon die Threading-Implementierung irgendwo intern nochmal eine Kopie beim Aufruf von thread_func (pass by value). Das reine Kopieren sollte eigentlich threadsicher sein und so arbeitest du auch nirgends auf den selben Instanzen bei der ganzen Kopiererei ("selbe Instanz" ist ja das Problem das zum Race führt).

DocShoe

Ja, so was hatte ich ja in meinem letzten Posting angedeutet.
Ich weiß allerdings nicht, ob das intern db-Connection Objekt auch thread-safe ist. Ich denke es ist besser, eine zweite Verbindung aufzumachen, die Zugangsdaten kann ich dem Original entnehmen.

Finnegan

@DocShoe sagte in Ist die Konstruktion eines shared_ptrs aus einem weak_ptr thread safe?:

Ja, so was hatte ich ja in meinem letzten Posting angedeutet.
Ich weiß allerdings nicht, ob das intern db-Connection Objekt auch thread-safe ist. Ich denke es ist besser, eine zweite Verbindung aufzumachen, die Zugangsdaten kann ich dem Original entnehmen.

Das könnte sogar vorteilhaft für die Performance sein. Je nachdem, wie der Server implementiert ist, könnte es durchaus sein, dass er jede "Connection" nur in einem Thread bearbeitet. Das hängt aber stark davon ab, wie der Server intern arbeitet. Es ist auch gut möglich, dass er Datenbank-Queries intern parallelisiert. Ich bin da bei Datenbanken nicht so drin, aber z.B. eine Webserver-Anfrage wird meistens von nur einem Thread aus einem Pool "bearbeitet".