(fast) gleichzeitig Elemente Vectoren hinzufügen

Ulf

Ich danke euch, damit hat alles geklappt.

Eisflamme

Man sollte halt zusätzlich Funktionen anbieten, welche mehrere Pushs nach Möglichkeit bündeln. Dann kann ein Thread ein paar sammeln und dann gemeinsam an einen atomic_push übergeben. Auf diese Weise wird die Anzahl der Locks verringert.

hustbaer

Man könnte auch z.B. die lock() und unlock() Funktionen public machen, und ne rekursive Mutex nehmen (z.B. Critical Section unter Windows oder boost::recursive_mutex).

Oder man macht es wie es alle machen, und lässt Mutex und Vektor einfach getrennt

GorbGorb

Ulf schrieb:

Hallo,
Im Allgemeinen laufen die Threads eine längere Zeit, da sie warten müssen, bis eine bestimmte Datei heruntergeladen wurde. Nachdem alle Threads fertig sind, greife ich auf die Objekte zu und das Programm stürzt gelegentlich ab.

Du fängst doch sowieso erst nachdem alle threads fertig sind damit an, den vector zu verwenden, oder? Ich würde dann lieber in jedem thread einen eigenen vector füllen, alle threads joinen und danach alle vectoren zusammenfügen. Damit vermeidest du mutex und (wenn die threads tatsächlich ansonsten alles asynchron machen) race conditions.
Wenn die Anzahl der geladenen Objekte bei jedem thread schon im Voraus bekannt sind, kannst du auch einen einzigen vector am Anfang auf die richtige Größe bringen und jedem thread einen entsprechenden Abschnitt zum Beschreiben mitgeben.

TyRoXx

cooky451 schrieb:

Mal simpler Beispielcode:

#include <atomic>
#include <mutex>
#include <future>
#include <thread>
#include <vector>
#include <iostream>

template <typename T, typename U>
void atomic_push_back(std::vector<T>& v, U&& var)
{
  static std::mutex m;
  std::lock_guard<std::mutex> guard(m);
  v.push_back(std::forward<U>(var));
}

void foo(std::vector<int>& v)
{
	for (int i = 500; i--; )
		atomic_push_back(v, i);
}

int main()
{
  std::vector<int> v;
  std::vector<std::thread> threads;
  for (int i = 100; i--; )
    threads.push_back(std::thread(foo, std::ref(v)));
  for (auto i = threads.begin(); i != threads.end(); ++i)
    i->join();
  for (auto i = v.begin(); i != v.end(); ++i)
    std::cout << *i << ' ';
}

std::vector<int> v;
//...

atomic_push_back(v, 0); //U ist int &&

//.. währenddessen in einem weit, weit entfernten Thread
const int i = 10;
atomic_push_back(v, i); //U ist const int & -> andere Funktion -> anderer Mutex -> bäm

Ein schönes Beispiel, warum static bei lokalen Variablen böse ist und Missbrauch von Gebrauch oft schwer zu unterscheiden ist.

314159265358979

cooky451 schrieb:

static std::mutex m;

Und wer garantiert dir, dass die Erstellung des Mutex atomar ist?

cooky451

TyRoXx schrieb:

Ein schönes Beispiel, warum static bei lokalen Variablen böse ist und Missbrauch von Gebrauch oft schwer zu unterscheiden ist.

Autsch. Wobei ich es eher als Beispiel dafür sehen würde, dass man bei Templates nicht vergessen sollte, dass das unterschiedliche Funktionen werden.

unskilled

314159265358979 schrieb:

cooky451 schrieb:
static std::mutex m;
Und wer garantiert dir, dass die Erstellung des Mutex atomar ist?

muss sie nicht. gcc kann es auf jeden fall; msvc weiß ich gerade nicht, sollte aber auch...

hustbaer

314159265358979 schrieb:

cooky451 schrieb:
static std::mutex m;
Und wer garantiert dir, dass die Erstellung des Mutex atomar ist?

Ob die Erstellung der Mutex atomar ist oder nicht, ist vollkommen egal.

Wenn nämlich die Chance besteht dass man auf die Mutex zugreift während diese gerade initialisiert wird, dann besteht genauso auch die Chance dass man auf die Mutex zugreift bevor überhaupt angefangen wurde sie zu initialisieren.

Ob die noch-nicht-angefangene Initialisierung atomar ist oder nicht spielt dann überhaupt keine Rolle mehr.

314159265358979

Und wenn der Mutex 2 mal initialisiert wird und dabei Daten des zuerst "fertig" erstellten Mutex überschrieben werden? :p

cooky451

Insgesamt war die Idee eines lokalen statischen mutex eigentlich ziemlicher Schwachsinn, wenn man sich nur mal überlegt, wie man das erweitern möchte. Da hat man dann ein atomic_pop_front() und die bekommt wieder ein extra mutex? Doppelt aua. Eine safe_vector* Klasse ist wohl doch die einzig passable Lösung. (Abgesehen natürlich von std::future mit std::async, was ich bevorzugen würde wenn es geht.)

* Und dann am besten auch gleich nicht vector nennen, sondern sondern nach etwas was besser passt. (Und dann auch ein verringertes Interface anbieten.) Sonst kommt noch jemand auf die Idee Algorithmen mit dem Vektor aufzurufen.

hustbaer

314159265358979 schrieb:

Und wenn der Mutex 2 mal initialisiert wird und dabei Daten des zuerst "fertig" erstellten Mutex überschrieben werden? :p

Ach, das ist ein function-local static.
Nachdem du den Kontext in deinem Zitat unterschlagen hast, war das nicht ersichtlich, und ich dachte das Ding wäre global.

Ja, function-local statics sind in pre-C++ 11 Zeiten ein potentielles thread-safety Problem.

Ulf

Hallo,

ich habe noch ein neues Problem in Anlehnung an diese Problematik.

Ich starte zwei Threads, die jeweils einen Pointer auf das gleiche Objekt erhalten. Um sicherzugehen, habe ich mal im ganzen Thread, außer während eines Download, Mutex gelocked.

Hier ein Auszug der Funktion, die als Thread ausgeführt wird:

//MUTEX BEGIN
	if (pthread_mutex_lock(&mutex) != 0){cout << "[FATAL] COULD NOT LOCK MUTEX";}

//irgendwelcher thread-unsicherer Code
		if (blabla){
//irgendwelcher thread-unsicherer Code
			if (pthread_mutex_unlock(&mutex) != 0){cout << "[FATAL] COULD NOT UNLOCK MUTEX";}
//MUTEX END
			etc_lib().download_file(url, localfile);
//MUTEX BEGIN
			if (pthread_mutex_lock(&mutex) != 0){cout << "[FATAL] COULD NOT LOCK MUTEX";}
		}
		try{
			string s = ArticleView->get_settingsreader()->get_value("temp_download_directory");
			s += ArticleView->get_article()->hash + ".htm";
			article* a = ArticleView->get_article();
			...

Ich sehe, dass der zweite Thread erst startet (bzw. in den unsicheren Code geht), wenn im ersten Thread der Download beginnt. Beide Downloads starten dann. Wenn ein Thread mit dem Download fertig ist, geht er weiter in den try Block, der andere Thread wartet. So sollte das ja auch sein.

Dummerweise stürzt der Programm immer in während der ersten beiden Zeilen (meist in der ersten) vom try-Block ab. An den Methoden kann es nicht liegen, ohne Parallelisierung klappt das. Allerdings nutzen beide Threads den gleichen Pointer ArticleView und damit auch das gleiche Objekt settingsreader. Die Methode settingsreader()->get_value() sucht in einem Member-Vector nach dem übergebenden Parameter.

Da beide Threads vor dem Download die Objekte genutzt haben, kann es sein, dass der eine Thread das Objekt noch irgendwie blockiert oder nutzt?

Habe ich da etwas grundsätzlich falsch verstanden?

Ich danke euch schonmal.

Ulf

hat niemand eine Idee?

pyhax

Doch, debugger nehmen und die !genaue! Stelle im Code finden. Ich tippe auf eine Dereferenzierung eines Nullpointers.

Ulf

Meist bricht er in der ersten Zeile des try-Blocks ab. Das ist die Methode:

string settings_reader::get_value(string setting_name)
{
	for (int i = 0; i < m_setting.size(); i++)   // m_settings ist ein vector<string>
		if (m_setting[i] == setting_name){   // Abbruch
			return m_values[i];
		}
	return "nA";
}

Beim Debuggen brach er einmal im zweiten Durchlauf, einmal beim ersten Durchlauf ab. Die Fehlermeldung(en): manchmal "*** Abgestürzt mit Rückgabewert: 0 ", manchmal " Programm hat Signal SIGSEGV (Segmentation fault) empfangen ***".

Auf dieses Objekt haben ja beide Threads Zugriff, bzw. vorher auch schon genutzt. Kann es da sein, dass der eine Thread, der eigentlich wartet, das Objekt, bzw. dessen vector noch blockiert?

pyhax

Ist i innerhalb der Größe des vectors?

Ulf

Wie ich schon gesagt habe, kann es nicht an der Methode selber liegen, da mit einem Thread alles läuft, der Fehler aber erst bei zwei gleichzeitigen Threads kommt. Die Threads selber ändern die Objekte nicht. (Mir fällt dabei auf, dass ich die Methode nicht als const definiert habe, ist gerade nachgeholt worden).

edit: hier stand Mist. Beide Vektoren haben die gleiche Größe, das ist garantiert.

pyhax

Mach doch einfach mal ein assert(m_setting.size() == m_values.size() ) an den Anfang.

Ulf

Ich habe mir die Größe der Vektoren auch explizit ausgeben lassen, beide sind immer gleich.