std::lock_guard (mutex) produziert deadlock

[Ein ehemaliger Benutzer]

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

Sprich, der mutex scheint bereits blockiert.

Nein, du fasst Speicher an, der dir nicht gehört.

Was allerdings die vier Zeilen Code mit dem Problem zu tun haben sollen, verstehe ich noch nicht ganz.

Schlechter Code = viele Fehler

[LynarStudios]

Nein, du fasst Speicher an, der dir nicht gehört.

Das vielleicht. Aber der mutex blockiert definitiv auch. Wenn ich try_lock auf den mutex anwende und ihn meinetwegen ein paar Mal in einer Schleife versuche zu lock'en, befinde ich mich in einer Endlosschleife. Das mag durch Speicherzugriffsverletzung begründet sein. Aber die Frage ist warum? Weil wie gesagt. Ich fasse den mutex ja nicht vorher an? Und die Funktion wird auch das erste Mal aufgerufen.

Schlechter Code = viele Fehler

Nein.

[Ein ehemaliger Benutzer]

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

Das vielleicht. Aber der mutex blockiert definitiv auch.

Zwecklos

[LynarStudios]

@manni66 sagte in std::lock_guard (mutex) produziert deadlock:

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

Das vielleicht. Aber der mutex blockiert definitiv auch.

Zwecklos

Sinnlos

[TGGC]

Wenn du try_lock auf irgendwelchen ungueltigen Speicher aufrufst, kann sonstwas passieren. Z.b. das es aussieht als waere es schon gelockt.

[Schlangenmensch]

Sind das std::mutex und std::lock_guard?

Bei sowas ist ein kompilierbares (minimal) Beispiel, welches den Fehler reproduziert sehr hilfreich, dann könnte man selbst mal 'nen Debugger drüber laufen lassen und gucken was genauer knallt und wodrauf evt. zugegriffen wird.

[LynarStudios]

@schlangenmensch

Hey!

Sind das std::mutex und std::lock_guard?

Ja der namespace ist std! Das sind also std::mutex und std::lock_guard!

Bei sowas ist ein kompilierbares (minimal) Beispiel, welches den Fehler reproduziert sehr hilfreich

Puh, das ist schwierig. Ich habe halt nur das engine Test Framework als VS2017 Projekt. Das könnte ich gerne hochladen? Das ist halt minimal nur die engine, in der man Engine Funktionen testen könnte. Dort gibt es zum Beispiel das kleine Testprogramm:

#include <iostream>
#include <thread>
using namespace std;
#include "LEMoon\include\le_moon.h"

int main(int argc, char ** argv)
{
  int result = LE_NO_ERROR;
  LEMoonInstance engine = new LEMoon();
  engine->init("Lynar Moon Engine");
  engine->initImage();

  engine->modelCreate(3);
  engine->modelAddDirection(3, 1, glm::vec2(1.0f, 1.0f));

  while(engine->pollEvent() || !result)
  {
    engine->beginFrame();
    engine->drawFrame();

    glm::vec2 dir = engine->modelGetDirection(3, 1);

    if(engine->keyEvent(SDL_KEYUP, SDLK_ESCAPE))
      {break;}

    engine->endFrame();
  }

  delete engine;
  exit(result);
}

Dieses Programm produziert den Fehler aber gar nicht Hier wird ja auch jede Iteration die fehleranfällige Funktion aufgerufen und es gibt kein Problem.

Das Problem tritt in dem Spieleprojekt auf. Da kann ich Dir auch gerne einen Zugang zu geben, aber das Projekt ist natürlich komplexer und viel viel größer

[theta]

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

std::mutex

std::mutex ist nicht rekursiv, d. h. wenn ein Thread den Mutex gelockt hat, kann dieser vom gleichen Thread nicht nochmal gelockt werden (vgl. dazu std::recursive_mutex). Vielleicht ist das die Ursache für die Blockade, die Access Violation rührt allerdings von einem anderen Problem her.

[LynarStudios]

@theta

Hey!

Also da ich immer noch neu in diesem Thema bin, sage ich einfach vielleicht mal, was mein Ziel ist. Nehmen wir mal die Funktion modelGetDirection(), die ja oben auch definiert ist, und angenommen ich habe 4 Threads laufen.

Jeder Thread ruft modelGetDirection() jeweils unterschiedlich oft auf, völlig egal wie oft, dann möchte ich aber, dass immer nur ein thread, meinetwegen thread nr 2 aktuell, modelGetDirection() aufruft. Die anderen threads sollen warten bis thread nr 2 fertig ist. Dann sollen die anderen threads sich meinetwegen drum kloppen wer als nächstes dran ist. Aber es ist immer nur ein thread dran mit dem Ausführen dieser Funktion.

Ist hier dann aber std::mutex_recursive die richtige Wahl? Ist das so zu verstehen, dass std::mutex_recursive.lock() quasi jedes Mal gestapelt wird und wieder vom Stack entfernt wird, wenn unlock() meinetwegen durch einen lock_guard aufgerufen wird? Ich bin mittlerweile leicht verwirrt...

[Schlangenmensch]

Um mir eine Engine + ein Spiel runter zu laden, zu kompilieren und zu debuggen fehlt mir im Moment die Zeit.
Wenn die Funktion in dem Beispiel Programm funktioniert, richt es fast, als ob du in dem Spiel die Engin nicht richtig initialisiert hast. Sollte dann aber nicht unbdeingt mit dem Mutex zusammen hängen. Aber bei UB kann's halt irgendwo krachen.

Ich würde mal im Debugger durch steppen und mir die Werte der einzelnen Member Variablen z.B. anschauen.

Wie wird level1ControlShip() denn aufgerufen. Da hast du ja so ein ekelhaften Void Pointer drin. Das ist ein Konstrukt, dass ich früher mal in der C - Netzwerkprogrammierung gebraucht habe, aber in C++ noch nie.

Ich habe grade mal in 2 source files der Engine geschaut. Meiner Meinung nach würden der ein paar moderne C++ Konstrukte auch ganz gut tun, aber das ist ja hier nicht das Thema.

[Schlangenmensch]

@lynarstudios
ein Thread kann std::mutex nur einmal locken. Wenn dann ein anderer Thread den Lock haben will muss er warten. Wenn die Sachen abgearbeitet sind, wird der lock entfernt (sonst deadlock).

Ein Thread kann eine mutex aber nicht 2 mal nacheinander locken ohne das der erste Lock im Programmablauf vorher aufgehoben wurde. Wenn man das doch benötigt, z.B. um eine Mutex in einer rekursiven Funktion aufzurufen, benötigt man std::mutex_recursive.

Dein Lockguard in modelGetDirection() wird nach abarbeiten der Funktion wieder zerstört und der Lock Freigegeben. Du darfst halt in den, von modelGetDirection() aufgerufenen Funktionen, die Mutex "this->mtxModel.modelGetDirection" nicht nochmal verwenden.

[theta]

@lynarstudios

Hier noch ein Beispiel zu den Erklärungen von @Schlangenmensch:

Im untenstehend Code wird in Foo::say_hello das 1. Mal gelockt, dann wird in Foo::name das 2. Mal versucht den Mutex zu locken, obwohl er schon gelockt ist (alles auf ein und demselben Thread). Das nur dann, wenn der Mutex "rekursiv" aufrufbar ist, im Falle von std::mutex blockiert das Programm.

#include <iostream>
#include <string>
#include <thread>
#include <mutex>

template <typename Mutex>
struct Foo
{
    void say_hello()
    {
        std::lock_guard<Mutex> lock{ m_mutex };
        std::cout << "Hello " << name() << "\n";
    }

    std::string name()
    {
        std::lock_guard<Mutex> lock{ m_mutex };
        return "lynarstudios";
    }

private:
    Mutex m_mutex;
};

int main()
{
    std::thread thread{ []
    {
        ///Foo<std::mutex> foo{};
        Foo<std::recursive_mutex> foo{};
        foo.say_hello();
    }};

    thread.join();
}

[wob]

Meine erste Vermutung war diese Zeile hier:

Parameter param = (Parameter) pointer;

Weil pointer ja ein Pointer ist, hätte ich hier bei Parameter einen Stern erwartet. Aber vermutlich ist das ein Pointer-typedef, denn es wird ja auf param->... zugegriffen. Warum dann nicht als Parameter direkt "Parameter" statt void* nehmen?

Ich würde aber sehr davon abraten, den * irgendwie zu verstecken.

Und dann frage ich mich noch, ob du speedLeft noch anderswo nutzt. Denn irgendwie erscheint es komisch, dass das jedes mal berechnet wird, die statische Konstante INCREASE_SPEED_LEFT aber dann vom Resultat gesetzt wird.

[LynarStudios]

@wob

Hey!

Der Parameter pointer ist etwas kompliziert zu erklären. Das hängt allerdings mit dem modularen Aufbau zusammen, der das zumindest bis jetzt leider notwendig macht. Dieser Parameter Pointer enthält Zeiger auf die engine, auf die game class und auf eine memory organizer Klasse. Diesen Zeiger gibt es halt, weil die Headereinbindungen sonst nicht funktionieren würden, weshalb er auch vom Typ void* als Parametertyp erst einmal sein muss.

speedLeft wird bei jedem Funktionsaufruf von level1ControlShip ausgelesen, wird aber nicht verändert. speedLeft wird in diversen Abfragen im Folgecode verwendet.

[Swordfish]

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

Diesen Zeiger gibt es halt, weil die Headereinbindungen sonst nicht funktionieren würden, weshalb er auch vom Typ void* als Parametertyp erst einmal sein muss.

. o O ( das riecht nach Quatsch mit Sauce )

[LynarStudios]

@schlangenmensch

Dann ist ja meine Herangehensweise eigentlich richtig:

Egal wie viele Threads es gibt. Geht irgendein Thread in diese Funktion rein, soll die Funktion für andere Threads blockiert werden, solange sie abgearbeitet wird.

Daher sollte

lock_guard<mutex> lockA(this->mtxModel.modelGetDirection);

Die richtige Herangehensweise sein, oder?

Ich würde mal im Debugger durch steppen und mir die Werte der einzelnen Member Variablen z.B. anschauen.

Habe ich gemacht. Mir sind keine fehlerhaften Werte aufgefallen:

•    this->mtxModel.mtxModelGetDirection    {...}    std::mutex
  

•    std::_Mutex_base    {_Mtx_storage={_Val=8.487983163861e-314#DEN _Pad=0x00007ff9190aea28 "" } }    std::_Mutex_base
  

•    _Mtx_storage    {_Val=8.487983163861e-314#DEN _Pad=0x00007ff9190aea28 "" }    std::_Align_type<double,80>
    _Val    8.487983163861e-314#DEN    double
  

•    _Pad    0x00007ff9190aea28 ""    char[80]
    [0]    0 '\0'    char
    [1]    0 '\0'    char
    [2]    0 '\0'    char
    [3]    0 '\0'    char
    [4]    4 '\x4'    char
    [5]    0 '\0'    char
    [6]    0 '\0'    char
    [7]    0 '\0'    char
    [8]    24 '\x18'    char
    [9]    -1 'ÿ'    char
    [10]    10 '\n'    char
    [11]    0 '\0'    char
    [12]    32 ' '    char
    [13]    0 '\0'    char
    [14]    0 '\0'    char
    [15]    0 '\0'    char
    [16]    0 '\0'    char
    [17]    0 '\0'    char
    [18]    0 '\0'    char
    [19]    0 '\0'    char
    [20]    1 '\x1'    char
    [21]    0 '\0'    char
    [22]    0 '\0'    char
    [23]    0 '\0'    char
    [24]    34 '\"'    char
    [25]    5 '\x5'    char
    [26]    -109 '“'    char
    [27]    25 '\x19'    char
    [28]    3 '\x3'    char
    [29]    0 '\0'    char
    [30]    0 '\0'    char
    [31]    0 '\0'    char
    [32]    88 'X'    char
    [33]    -1 'ÿ'    char
    [34]    10 '\n'    char
    [35]    0 '\0'    char
    [36]    0 '\0'    char
    [37]    0 '\0'    char
    [38]    0 '\0'    char
    [39]    0 '\0'    char
    [40]    0 '\0'    char
    [41]    0 '\0'    char
    [42]    0 '\0'    char
    [43]    0 '\0'    char
    [44]    7 '\a'    char
    [45]    0 '\0'    char
    [46]    0 '\0'    char
    [47]    0 '\0'    char
    [48]    112 'p'    char
    [49]    -1 'ÿ'    char
    [50]    10 '\n'    char
    [51]    0 '\0'    char
    [52]    80 'P'    char
    [53]    0 '\0'    char
    [54]    0 '\0'    char
    [55]    0 '\0'    char
    [56]    0 '\0'    char
    [57]    0 '\0'    char
    [58]    0 '\0'    char
    [59]    0 '\0'    char
    [60]    1 '\x1'    char
    [61]    0 '\0'    char
    [62]    0 '\0'    char
    [63]    0 '\0'    char
    [64]    34 '\"'    char
    [65]    5 '\x5'    char
    [66]    -109 '“'    char
    [67]    25 '\x19'    char
    [68]    1 '\x1'    char
    [69]    0 '\0'    char
    [70]    0 '\0'    char
    [71]    0 '\0'    char
    [72]    -48 'Ð'    char
    [73]    -1 'ÿ'    char
    [74]    10 '\n'    char
    [75]    0 '\0'    char
    [76]    0 '\0'    char
    [77]    0 '\0'    char
    [78]    0 '\0'    char
    [79]    0 '\0'    char
  

Ich habe grade mal in 2 source files der Engine geschaut. Meiner Meinung nach würden der ein paar moderne C++ Konstrukte auch ganz gut tun, aber das ist ja hier nicht das Thema.

Für so etwas bin ich immer offen. Ich stehe nur leider etwas unter Zeitdruck und sollte das Problem zuerst angehen.

Wie wird level1ControlShip() denn aufgerufen.

param->game->level1ControlShip() ist der Funktionsaufruf in einer übergeordneten Funktion. Wenn ich übrigens

param->engine->modelGetDirection(MODEL_VERA, LEFT);

in level1ControlShip() auskommentiere, gibt es dort natürlich keinen Fehler. Darunter kommt dann aber irgendwann die Zeile:

param->engine->modelGetPositionD(MODEL_VERA);

Diese Funktion spinnt ebenfalls an der gleichen Stelle, nämlich beim Lock Guard:

glm::vec2 LEMoon::modelGetPositionD(uint32_t id)
{
  lock_guard<mutex> lockA(this->mtxModel.modelGetPositionD);
  glm::vec2 position;
  LEModel * pElem = this->modelGet(id);

  if(pElem == nullptr)
    {pElem = this->modelGetFromBuffer(id);}

  if(pElem != nullptr)
    {position = pElem->pModel->mdlGetPositionD();}
  else
  {
    #ifdef LE_DEBUG
      char * pErrorString = new char[256 + 1];
      sprintf(pErrorString, "LEMoon::modelGetPositionD(%u)\n\n", id);
      this->printErrorDialog(LE_MDL_NOEXIST, pErrorString);
      delete [] pErrorString;
    #endif
  }

  return position;
}

Das hatte ich vorher nicht bemerkt.

[LynarStudios]

@swordfish

Cocktail oder Barbecue Sauce?

[theta]

Zeige mal die Definition von Parameter und wie die Funktion Game::level1ControlShip aufgerufen wird.

[Swordfish]

@lynarstudios sagte in std::lock_guard (mutex) produziert deadlock:

@swordfish

Cocktail oder Barbecue Sauce?

missing forward-declaration-sauce.

[Schlangenmensch]

Generell sehe ich kein Fehler in deiner Verwendung von der Mutex, nach wie vor glaube ich, dass mirgendwas nicht initialisiert ist.

Was spricht denn gegen:

void Game::level1ControlShip(Parameter param, bool controlAble)
{
...
glm::vec2 speedLeft = param->engine->modelGetDirection(MODEL_VERA, LEFT);
...
}

dein Parameter wird offenbar irgendwo ander definiert und ist ein Typedef auf ein Pointer, was recht verwirrend ist. Aber den Typ solltest du auch in der Funktionsdefinition verwenden können.

Funktioniert der Code denn Singlethreaded?

P.S: Gibt es einen Grund dafür, dass du konstant C-Strings verwendest, anstelle von std::string?