Variablen Initialisierung mit lock_guard



  • Hi zusammen,

    ich habe einen shared ptr, der parallel geschrieben und gelesen werden könnte:

    std::shared_ptr<Foo> non_thread_safe_ptr;
    

    Ich habe nun einen Code Auschnitt, in denen diese ptr nur gelesen werden muss. Brauche die Daten aus Foo und möchte verschiedene Sachen mit denen machen. Um nicht einen riesigen Abschnitt zu sperren, sieht das aktuell ungefähr so aus:

    ...
    FooMember1 member1; 
    FooMember2 member2; 
    FooMember3 member3; 
    {
    std::lock_guard<std::mutex> lock(some_mutex); 
    member1 = non_thread_safe_ptr->member1; 
    member2 = non_thread_safe_ptr->member2; 
    member3 = non_thread_safe_ptr->member3; 
    }
    ...
    
    // Hier viele Dinge mit member1, member2, member3 machen
    

    Irgendwie gefällt mir das nicht so richtig. Das erinnert mich immer an alten C Code, wo man am Anfang alle Variablen schreibt 😃
    Darüber hinaus kommt es mir umständlich vor und ist auch problematisch, wenn z.B. Member1, Member2, Member3 nicht default initialisierbar sind.

    Was habe ich für Alternativen?

    Eine Alternative ist natürlich alles in den mutex zu holen, Code ist dann sehr schön finde ich, aber natürlich nicht allzu performant.

    Die zweite Idee die ich hatte war es eine Kopie vom ptr zu erstellen:

    ...
    std::shared_ptr<Foo> copyFoo; 
    {
    std::lock_guard<std::mutex> lock(some_mutex); 
    copyFoo = std::make_shared<Foo>(non_thread_safe_ptr);
    }
    ...
    

    Evtl. kann man das dann in ne Funktion auslagern:

    std::shared_ptr<Foo> copyFoo = getFooCopy();
    

    Oder als Lambda?

    std::shared_ptr<Foo> copyFoo {
    [&] {
            std::lock_guard<std::mutex> lock(some_mutex);
            return std::make_shared<Foo>(non_thread_safe_ptr);
        } ()
    }
    

    -> Klappt das?

    Gibt es noch andere Möglichkeiten? Eine Kopie vom ganzen Objekt zu erstellen ist ja evtl. auch nicht möglich oder nicht sinnvoll, wenn nur eine wenige Daten benötigt werden.



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    FooMember1 member1; 
    FooMember2 member2; 
    FooMember3 member3; 
    {
    std::lock_guard<std::mutex> lock(some_mutex); 
    member1 = non_thread_safe_ptr->member1; 
    member2 = non_thread_safe_ptr->member2; 
    member3 = non_thread_safe_ptr->member3; 
    }
    

    Das wäre eine Möglichkeit:

    class Foo {
    public:
        Foo(T* non_thread_safe_ptr, std::mutex& mutex) : Foo(non_thread_safe_ptr, std::lock_guard<std::mutex>{mutex}) {
        }
    
    private:
        Foo(T* non_thread_safe_ptr, std::lock_guard<std::mutex> const&) : 
                member1{non_thread_safe_ptr->member1},
                member2{non_thread_safe_ptr->member2},
                member3{non_thread_safe_ptr->member3} {
        }
    
        FooMember1 member1; 
        FooMember2 member2; 
        FooMember3 member3; 
    }
    

    Wenn Foo viele weitere Member hat, und der Lock nicht während der Konstruktion dieser gehalten werden soll, dann würde ich empfehlen alles was den Lock benötigt in eine eigene kleine Hilfs-struct zu verpacken, und diese Hilfs-struct dann als Member von Foo zu halten. Dann kannst du den selben Trick mit den zwei Konstruktoren mit der Hilfs-struct machen.



  • @hustbaer Zum Verständis: Das wäre jetzt die Alternative, wenn Foo viele Member hat, ich aber nur einige davon benötige, oder?

    Wenn ich alle benötige, dann wäre mein Ansatz von oben mit dem kopieren des Objektes besser, weil kürzer? (Und vorrausgesetzt kopieren funktioniert)



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    @hustbaer Zum Verständis: Das wäre jetzt die Alternative, wenn Foo viele Member hat, ich aber nur einige davon benötige, oder?

    Genau.

    Wenn ich alle benötige, dann wäre mein Ansatz von oben mit dem kopieren des Objektes besser, weil kürzer? (Und vorrausgesetzt kopieren funktioniert)

    Kommt drauf an, ich kenne ja deinen Code nicht.

    Wenn's dir um kurz geht kannst du es aber dennoch anders machen: du kannst z.B. die Member in Foo alle in einen struct stecken, und dann den ganzen struct auf einmal im ctor von Foo kopieren. Also so wie in meinem 1. Beispiel, bloss halt mit nur einem Member statt 3, und dieses eine Member ist dann ne struct wo alle Membervariablen drin sind die du brauchst.

    Ich würde das aber nicht von "kurz" abhängig machen. Was Sinn macht kommt auf viele Faktoren an die ich alle nicht kenne. Wenn du etwas mehr Code herzeigst, inklusive Semantik und dem drumrum um die Klasse, kann man vielleicht was konkreteres sagen.

    Also speziell wundert mich schonmal dass du in einer Memberfunktion von Foo nen externen Mutex lockst. Das ist komisch. Oder ist der Mutex doch eingebaut? Alles etwas unklar.



  • @hustbaer Vlt. war meine Ausführung tatsächlich etwas zu kurz, um nachvollziehbar zu sein.

    Ich habe eine Klasse, die sowohl als Publisher als auch subscriber fungiert. Daher es gibt zwei Methoden, eine zum empfangen und eine zum senden, beide werden in festen Zeitintervallen (und ggf. parallel) aufgerufen.
    Die subscribe Methode empfängt Objekt der Klasse Foo und speichert diese in einer Member variable. Die publisher Methode verarbeitet dieses Objekt und erstellt daraus die Message, die gepublisht wird.
    Diese Struktur ist mehr oder weniger in den Stein gemeißelt, daher die Verarbeitung direkt in die subscribe Methode zu ziehen geht nicht.

    Der Zugriff auf das Objekt ist also durch ein Mutex geschützt, dieser ist auch Teil der Klasse.

    In der publisher Methode ist es aktuell so, dass es einen Abschnitt gibt, der durch mutex gesperrt wird. In diesem werden einige Daten des Objektes in Variablen zwischengespeichert (ggf. mit noch extra Bearbeitung), um sie in späteren Teilen der Methode noch zu verwenden. Also so wie ich das im Eingangspost schematisch dargestellt habe. Nur das halt in dem lock guard Teil etwas länger ist.

    Man hört es vlt. raus: Die Funktion ist aktuell nicht so ganz optimal und ich versuche etwas Ordnung in das Ganze zu bringen. Primär erstmal dadurch, dass ich Teile des Code logisch zusammenführe und in Untermethoden auslagere etc.
    Der Mutex macht mir das aktuell noch etwas schwierig, weil mehrfach locken nicht in Frage kommt (Subscriber Frequenz zu schnell).

    std::shared_ptr<Foo> copyFoo; 
    {
    std::lock_guard<std::mutex> lock(some_mutex); 
    copyFoo = std::make_shared<Foo>(non_thread_safe_ptr);
    }
    

    Ich vermute nach Überlegen, dass vermutlich diese Variante die beste ist für meinen konkreten Anwendungsfall. Mich hatte nur auch generell interessiert, was es noch für Alternativen gibt. Deine gefällt mir da auch sehr gut.

    Ich hatte grade auch noch die Idee, ob man vlt. std::atomic verwenden kann, um das ganze noch etwas einfacher zu machen. Die Vorraussetzungen für std::atomic sind in dem Code glaube ich erfüllt



  • Grundsätzlich is der ptr auch const ist mir grade aufgefallen (bzw. habe ich jetzt ergänzt).

    std::shared_ptr<const Foo> non_thread_safe_ptr;
    

    Ich dachte, ich könnte vlt. auch einfach mir das Objekt geben lassen. Soweit ich allerdings weiß ist dereferenzieren nicht atomic. Ich hatte daher folgende Idee:

    const auto underlyingObject = [&]{
            std::lock_guard<std::mutex> lock(mutex);
            return *non_thread_safe_ptr;
    }();
    

    Klaptt das oder gibt es hier noch Race Conditions? Geht es noch besser? 🙂

    bzw. das auch mit shared ptr?:

    const auto copy = [&]{
            std::lock_guard<std::mutex> lock(mutex);
            return std::make_shared(non_thread_safe_ptr);
    }();
    

    Da non_thread_safe_ptr auch ein nullptr sein kann



  • Boah, die Beschreibung ist immer noch ziemlich wirr. Du schreibst da öfter "die Klasse", "das Objekt" etc. - wo nicht 100% klar ist welche Klasse/welches Objekt (Foo oder die Publisher/Subscriber Klasse deren Namen du nicht verrätst?).

    Ich glaube aber die Lösung ist so einfach wie sie einfacher nicht sein könnte: du kopierst einfach nur den shared_ptr<const Foo>.

    const auto copy = [&]{
            std::lock_guard<std::mutex> lock(mutex);
            return non_thread_safe_ptr;
    }();
    

    Oder kann *non_thread_safe_ptr irgendwo geändert werden (also das Objekt auf das non_thread_safe_ptr zeigt, nicht der non_thread_safe_ptr selbst)? Wenn ja, dann wäre das schlecht. Wenn nein, dann sollte das so gehen.



  • @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Boah, die Beschreibung ist immer noch ziemlich wirr. Du schreibst da öfter "die Klasse", "das Objekt" etc. - wo nicht 100% klar ist welche Klasse/welches Objekt (Foo oder die Publisher/Subscriber Klasse deren Namen du nicht verrätst?).

    Ich glaube aber die Lösung ist so einfach wie sie einfacher nicht sein könnte: du kopierst einfach nur den shared_ptr<const Foo>.

    const auto copy = [&]{
            std::lock_guard<std::mutex> lock(mutex);
            return non_thread_safe_ptr;
    }();
    

    Oder kann *non_thread_safe_ptr irgendwo geändert werden (also das Objekt auf das non_thread_safe_ptr zeigt, nicht der non_thread_safe_ptr selbst)? Wenn ja, dann wäre das schlecht. Wenn nein, dann sollte das so gehen.

    Sorry! Würde am liebsten den Code ja reinposten, aber das geht nicht. An den textuellen Beschreibungen / Beispielcode werde ich noch arbeiten in Zukunft 😃

    Oh ja stimmt, geht natürlich so auchm klar. Nein das unterliegende Objekt wird nicht geändert, es ist const (Siehe Ergänzung letzter Post).

    Interessenshalber: Auch eine Zuweisung ist keine atomic Operation? Daher const auto copy = non_thread_safe_ptr geht nicht, richtig?
    Mein Verständnis ist: Zwei verschiedene Shared ptr Objekte sind thread safe (auch wenn sie auf das selbe Objekt zeigen). Daher die shared ptr Magie selbst ist thread safe, wenn z.B. ein shared ptr zerstört wird und der counter dekrementiert wird.
    Alles was aber auf der selben shared ptr Instanz passiert (schreiben, dereferenzieren, kopieren, auf nullptr prüfen etc.) ist nicht thread safe, korrekt?

    Mit https://en.cppreference.com/w/cpp/memory/shared_ptr/atomic2 könnte man sich in dem Fall dann das Mutex hier sparen, korrekt?

    Könnte man dann aktuell (nicht C++20) es auch so lösen:

    const auto copy = std::atomic_load(&non_thread_safe_ptr);
    


  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    Oder kann *non_thread_safe_ptr irgendwo geändert werden (also das Objekt auf das non_thread_safe_ptr zeigt, nicht der non_thread_safe_ptr selbst)? Wenn ja, dann wäre das schlecht. Wenn nein, dann sollte das so gehen.

    Oh ja stimmt, geht natürlich so auchm klar. Nein das unterliegende Objekt wird nicht geändert, es ist const (Siehe Ergänzung letzter Post).

    Aus deiner Ergänzung geht nur hervor dass das Publisher/Subscriber Objekt nur einen const Zeiger hat. Aber nicht dass es keine andere Komponenten gibt die einen non-const Zeiger auf das selbe Objekt haben und es ändern könnten.

    Interessenshalber: Auch eine Zuweisung ist keine atomic Operation? Daher const auto copy = non_thread_safe_ptr geht nicht, richtig?

    Richtig.

    Mein Verständnis ist: Zwei verschiedene Shared ptr Objekte sind thread safe (auch wenn sie auf das selbe Objekt zeigen). Daher die shared ptr Magie selbst ist thread safe, wenn z.B. ein shared ptr zerstört wird und der counter dekrementiert wird.
    Alles was aber auf der selben shared ptr Instanz passiert (schreiben, dereferenzieren, kopieren, auf nullptr prüfen etc.) ist nicht thread safe, korrekt?

    Korrekt.

    Mit https://en.cppreference.com/w/cpp/memory/shared_ptr/atomic2 könnte man sich in dem Fall dann das Mutex hier sparen, korrekt?

    Korrekt. Wobei du davon ausgehen kannst dass so ein atomic<shared_ptr<T>> nicht "lock free" ist. D.h. der wird intern erst wieder etwas wie einen Lock-Pool verwenden. Wobei dann meistens Spin-Locks verwendet werden. D.h. es kann sein dass der Overhead etwas geringer wird, aber viel wird sich da nicht tun.

    Könnte man dann aktuell (nicht C++20) es auch so lösen:

    const auto copy = std::atomic_load(&non_thread_safe_ptr);
    

    Ja, allerdings nur wenn du an allen Stellen ausschliesslich mit den atomic_xxx Funktionen auf den shared_ptr zugreifst.



  • @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Aus deiner Ergänzung geht nur hervor dass das Publisher/Subscriber Objekt nur einen const Zeiger hat. Aber nicht dass es keine andere Komponenten gibt die einen non-const Zeiger auf das selbe Objekt haben und es ändern könnten.

    Ah ja. Die gibt es tatsächlich nicht. Das Framework nutzt leider überall shared ptr für keinen offensichtlichen Grund.

    Daher der subscriber callback liefert einen shared pointer als Parameter, der aktuell intern ebenfalls als shared ptr gespeichert wird. Evtl. wäre es auch eine Option sich hier gegen das Framework aufzulehnen und selbst nicht shared ptr zu übernutzen.

    Dann könnte man auch hier mit atomic arbeiten.

    @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Korrekt. Wobei du davon ausgehen kannst dass so ein atomic<shared_ptr<T>> nicht "lock free" ist. D.h. der wird intern erst wieder etwas wie einen Lock-Pool verwenden. Wobei dann meistens Spin-Locks verwendet werden. D.h. es kann sein dass der Overhead etwas geringer wird, aber viel wird sich da nicht tun.

    Mir geht es insgesamt auch mehr darum den Code schön zu machen. Wenn er dann auch noch etwas performanter ist, dann natürlich umso besser!

    @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Ja, allerdings nur wenn du an allen Stellen ausschliesslich mit den atomic_xxx Funktionen auf den shared_ptr zugreifst.

    Das ist machbar.



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    @hustbaer sagte in Variablen Initialisierung mit lock_guard:
    Daher der subscriber callback liefert einen shared pointer als Parameter, der aktuell intern ebenfalls als shared ptr gespeichert wird. Evtl. wäre es auch eine Option sich hier gegen das Framework aufzulehnen und selbst nicht shared ptr zu übernutzen.

    Wieso? Ist in dem Fall doch super praktisch dass es ein shared_ptr ist.

    Dann könnte man auch hier mit atomic arbeiten.

    Verstehe ich nicht. Wie stellst du dir das vor? Willst du nen atomic<Foo> machen oder wie? Da ist shared_ptr<const Foo> fast sicher besser.

    atomic<Foo> macht mMn. nur Sinn wenn atomic<Foo>::is_always_lock_free ist. Was voraussetzt dass Foo nicht zu gross ist (üblicherweise max. 2x die Grösse eines Zeigers, auf manchen Plattformen aber auch nur 1x die Grösse eines Zeigers). Und dass Foo trivial kopierbar ist.



  • @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Verstehe ich nicht. Wie stellst du dir das vor? Willst du nen atomic<Foo> machen oder wie? Da ist shared_ptr<const Foo> fast sicher besser.
    atomic<Foo> macht mMn. nur Sinn wenn atomic<Foo>::is_always_lock_free ist. Was voraussetzt dass Foo nicht zu gross ist (üblicherweise max. 2x die Grösse eines Zeigers, auf manchen Plattformen aber auch nur 1x die Grösse eines Zeigers). Und dass Foo trivial kopierbar ist.

    Kannst du das näher erläutern? Also den ersten Teil, dass es trivial kopierbar ist mir bekannt. Ich konnte nicht ganz nachvollziehen, was "is_always_lock_free" bedeutet und waurm das wichtig wäre.



  • @Leon0402 Hab jetzt nicht alles gelesen. Kann es sein, dass du sowas suchst? https://www.grimm-jaud.de/index.php/blog/reader-writer-locks



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    Ich konnte nicht ganz nachvollziehen, was "is_always_lock_free" bedeutet und waurm das wichtig wäre.

    Hardware-Atomics stehen nur bis zu einer bestimmten Grösse zur Verfügung. Für grössere Dinge werden Locks verwendet um die Zugriffe atomar zu machen. Und dann kann man mMn. auch gleich selbst Locks verwenden.



  • @Tyrdal sagte in Variablen Initialisierung mit lock_guard:

    @Leon0402 Hab jetzt nicht alles gelesen. Kann es sein, dass du sowas suchst? https://www.grimm-jaud.de/index.php/blog/reader-writer-locks

    Nicht wirklich. Es ist ja alles aktuell thread safe. Es ging nur darum wie man den Code lesbarer bekommt.

    @hustbaer sagte in Variablen Initialisierung mit lock_guard:

    Hardware-Atomics stehen nur bis zu einer bestimmten Grösse zur Verfügung. Für grössere Dinge werden Locks verwendet um die Zugriffe atomar zu machen. Und dann kann man mMn. auch gleich selbst Locks verwenden.

    Naja es ist halt lesbarer und spart Codezeilen. Ist das kein Argument für dich?

    const auto copy = [&]{
            std::lock_guard<std::mutex> lock(mutex);
            return non_thread_safe_ptr;
    }();
    

    vs.

    const auto copy = non_thread_safe;
    

    Was für mich dagegen spricht evtl. ist die Gefahr, dass man nicht mehr so richtig merkt, warum man eig. hier dann die Kopie macht etc.
    Aber bei beiden Lösungen sehe ich eig. eine ähnliche Gefahr, dass man einfach in größerem Code versehntlich doch eine Variable ohne Lock verwendet oder anderweitige Fehler macht.

    Oder was ist der Vorteil den du hier siehst, selbst Locks zu verwenden? -> Im allgemeinen Fall ist man vlt. etwas flexibler würde mir jetzt einfallen. In diesem speziellen Fall ist aber 1x schreiben, 1x lesen ja exakt, was ich möchte.



  • @Leon0402

    Evtl. wäre es auch eine Option sich hier gegen das Framework aufzulehnen und selbst nicht shared ptr zu übernutzen.
    Dann könnte man auch hier mit atomic arbeiten.

    Ich habe das so verstanden dass du einen atomic<Foo> statt eines atomic<shared_ptr<const Foo>> machen willst. Bei atomic<shared_ptr<const Foo>> hab ich weniger Bedenken.

    atomic<Foo> kommt mir aber immer etwas komisch vor. Alleine schon wegen der Einschränkungen. Es reicht ja z.B. nicht dass Foo trivial kopierbar ist. Es darf z.B. auch kein Padding enthalten. Wobei reine Loads und Stores vermutlich trotzdem immer funktionieren werden, aber bei allen read-modify-write Operationen wird es dann doof sobald Padding vorhanden ist. Ab einer bestimmten Grösse wird es dann auch effizienter einen atomic<shared_ptr<const Foo>> zu verwenden. Weil dabei halt nicht immer das ganze Objekt kopiert werden muss. Wobei man da vermutlich schon einige hudert wenn nicht sogar einige tausend Byte braucht um mit atomic<shared_ptr<const Foo>> schneller zu sein.



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    vs.

    const auto copy = non_thread_safe;
    

    Was für mich dagegen spricht evtl. ist die Gefahr, dass man nicht mehr so richtig merkt, warum man eig. hier dann die Kopie macht etc.

    Meinst du

    const auto copy = thread_safe.load();
    

    ?
    Weil so wie du das geschrieben hast macht das für mich keinen Sinn. Weil 1) atomics nicht kopierbar sind und 2) der Name non_thread_safe dann nicht mehr passt. Ansonsten hab ich vermutlich nicht verstanden was du damit ausdrücken wolltest.



  • @Leon0402 sagte in Variablen Initialisierung mit lock_guard:

    Aber bei beiden Lösungen sehe ich eig. eine ähnliche Gefahr, dass man einfach in größerem Code versehntlich doch eine Variable ohne Lock verwendet oder anderweitige Fehler macht.

    Das lässt sich einfach lösen indem man Mutex + Datenstruktur in eine eigene Klasse verpackt die das kapselt.


Anmelden zum Antworten