Klassenfunktion mit Parameter als Thread

[Schlangenmensch]

@no_name1991 Du wartest darauf das tlogging beendet wird, bevor du addmessage aufrufst, start kommt aber nie zurück, daher wird addmessage auch schon nicht aufgerufen.

[no_name1991]

@Schlangenmensch
Vielen Dank für die Erklärung.

Wie muss man den Code verändern, wenn man vsmessage etwas hinzufügen möchte und der Thread parallel abgearbeitet werden soll?

Vielen Dank im Voraus.

[Helmut.Jakoby]

Hallo @no_name1991 ,
in einem Projekt habe ich mich mal mit einem Threads beschäftigt, der über eine Message-Queue Nachrichten bekam, die dieser abgearbeitet hat. War die Queue abgearbeitet, wurde der Threads "schlafen gelegt". Wurde etwas in die Queue abgelegt, wurde der Threads "aufgeweckt". Das Einfügen und Entnehmen aus der Queue wurde mit einem Mutex abgesichert. Wärend der Threadarbeit konnten andere Threads etwas in die Queue einfügen. Das ganze ist mMn nur sinnvoll, wenn der Threads auch gut zu tun hat.
Du solltest prüfen, ob eine einfache Ausgabe im Ablauf ohne zusätzlichen Thread nicht sinnvoller wäre.

[Schlangenmensch]

@no_name1991 Du könntest das tlogging.join(); ans Ende der main() setzen, ähnlich dem, was ich dir oben schon im Postskriptum vorgeschlagen habe.

Prinzipiell solltest du dir std::jthread anschauen. Das ist eine bessere Variante, die bei Zerstörung des Thread Objekts automatisch Joint.

Außerdem solltest du dir std::lock_guard anschauen, und damit die Locks realisieren, damit kannst du kein unlock vergessen.

Wenn dein eigentliches Anliegen ist, den Logger Asynchron zu haben, könnte das auch ein Anwendungsfall für eine Coroutine sein, die sind in C++ aber noch ziemliches Neuland.

[Lupus-SLE]

@no_name1991 sagte in Klassenfunktion mit Parameter als Thread:

Mein Ziel ist es, Ein/-Ausgaben, Zustände etc auszugehen bzw diese eventuell auf Festplatte zu speichern.
Dies soll dann das restliche Programm nicht blockieren

Das ist aus meiner Sicht ein valides Szenario (oder Use-Case bzw. User-Story), und es riecht nach dem klassischen Producer-Cosumer-Prinzip, wie es auch schon Jakoby beschrieb.

Möglicherweise genügt es aber auch, wenn es nur einen Cosumer gibt - also keine Warteschlange. Dann läufst du aber in die Gefahr, dass bei sehr vielen Logging-Aufrufen und langsamem Speichermedium eben doch blockiert wird.

Beim klassischen Producer-Consumer (in Intervallen) muss sichergestellt werden, dass das Put und Take nicht gleichzeitig passieren darf (MutEx bzw. Synchronisation). Es gibt auch die Möglichkeit, dass der Consumer wartet und vom Producer benachrichtigt wird ... oder sich beide gegenseitig benachrichtigen, dann entsteht aber immer die Dead-Lock-Möglichkeit, die ausgeschlossen werden muss. Für den Anfang würde ich in Intervallen checken, ob es etwas zu loggen gibt, und dann wieder je eine feste oder dynamische Zeit schlafen.

[Quiche Lorraine]

@Schlangenmensch sagte in Klassenfunktion mit Parameter als Thread:

Außerdem solltest du dir std::lock_guard anschauen, und damit die Locks realisieren, damit kannst du kein unlock vergessen.

Clang-tidy hat mich vor kurzem auf std::scoped_lock hingewiesen:

https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-scoped-lock.html

https://en.cppreference.com/w/cpp/thread/scoped_lock.html

Der Vorteil des Ganzen:

If several mutexes are given, deadlock avoidance algorithm is used as if by std::lock.

[Lupus-SLE]

So, hab mich mal in die Materie begeben. So könnte es gehen:

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

class AsyncLogging {
private:
	std::mutex mutex;
	std::thread logging_thread;
	std::vector<std::string> messages;
	int isRunning = 1;
	int isUpdated = 0;
public:
	void start() {
		logging_thread = std::thread(&AsyncLogging::run, this);
		std::cout << "logging thread started" << std::endl;
	}

	void end() {
		mutex.lock();
		isRunning = 0;
		mutex.unlock();
		logging_thread.join();
	}

	void run() {
		mutex.lock();
		while (isRunning || isUpdated) {
			if (isUpdated) {
				std::vector<std::string> messages_copy = messages;
				messages.clear();
				isUpdated = 0;
				mutex.unlock();

				for (auto s : messages_copy) {
					std::cout << s << " wird jetzt geloggt ..." << std::endl;
				}
			} else {
				mutex.unlock();
			}
			std::this_thread::sleep_for(std::chrono::milliseconds(100));
		}
	}

	void addMessage(std::string msg) {
		mutex.lock();
		messages.push_back(msg);
		isUpdated = 1;
		mutex.unlock();
	}
};

int main() {
	AsyncLogging logging;
	logging.start();

	for (int i = 1; i <= 10; i++) {
		logging.addMessage(std::to_string(i) + ". Aufruf");
	}

	logging.end();
	return 0;
}

In Zeile 37 kommt dann deine aufwändige Logging-Operation.

An für sich sollte es hier keinen Deadlock geben.(?)

Edit: Sorry, noch etwas falsch gemacht:

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

class AsyncLogging {
private:
	std::mutex mutex;
	std::thread logging_thread;
	std::vector<std::string> messages;
	int isRunning = 1;
	int isUpdated = 0;
public:
	void start() {
		logging_thread = std::thread(&AsyncLogging::run, this);
		std::cout << "logging thread started" << std::endl;
	}

	void end() {
		mutex.lock();
		isRunning = 0;
		mutex.unlock();
		logging_thread.join();
	}

	void run() {
		while (1) {
			mutex.lock();
			if (isUpdated) {
				std::vector<std::string> messages_copy = messages;
				messages.clear();
				isUpdated = 0;
				mutex.unlock();

				for (auto s : messages_copy) {
					std::cout << s << " wird jetzt geloggt ..." << std::endl;
				}
			} else if (!isRunning) {
				mutex.unlock();
				break;
			}
			mutex.unlock();
			std::this_thread::sleep_for(std::chrono::milliseconds(100));
		}
	}

	void addMessage(std::string msg) {
		mutex.lock();
		messages.push_back(msg);
		isUpdated = 1;
		mutex.unlock();
	}
};

int main() {
	AsyncLogging logging;
	logging.start();

	for (int i = 1; i <= 10; i++) {
	    std::this_thread::sleep_for(std::chrono::milliseconds(25));
		logging.addMessage(std::to_string(i) + ". Aufruf");
		std::this_thread::sleep_for(std::chrono::milliseconds(25));
	}

	logging.end();
	return 0;
}

[Lupus-SLE]

Dieser Beitrag wurde gelöscht!

[Schlangenmensch]

Da @Lupus-SLE offenbar auch alles, was hier geschrieben wurde, ignoriert, hier das ganze etwas überarbeitet:

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

class AsyncLogging {
  std::mutex mutex;
  std::jthread logging_thread;
  std::queue<std::string> messages;
  std::ostream& stream;
  std::atomic_bool end{ false };

  void do_logging()
  {
    std::lock_guard<std::mutex> lock(mutex);
    for (; !messages.empty(); messages.pop())
      stream << messages.front();
  }

  void stopLoggingThread()
  {
    end = true;
  }

  void run()
  {
    while (true)
    {
      if (end)
        return;

      do_logging();
      std::this_thread::sleep_for(std::chrono::milliseconds(100));
    }
  }

public:
  AsyncLogging(std::ostream& stream):
    logging_thread(&AsyncLogging::run, this),
    stream(stream)
  {
    std::cout << "logging thread started\n";
  }

  ~AsyncLogging()
  {
    stopLoggingThread();
    do_logging();
    std::cout << "logging thread ended\n";
  }

  void addMessage(std::string msg) 
  {
    std::lock_guard<std::mutex> lock(mutex);
    messages.push(msg);
  }
};

int main() {
  AsyncLogging logging(std::cout);

  for (int i = 1; i <= 10; i++) 
  {
    std::this_thread::sleep_for(std::chrono::milliseconds(25));
    logging.addMessage(std::to_string(i) + ". Aufruf  \n");
    std::this_thread::sleep_for(std::chrono::milliseconds(25));
  }

  return 0;
}

[Lupus-SLE]

@Schlangenmensch sagte in Klassenfunktion mit Parameter als Thread:

std::lock_guard<std::mutex> lock(mutex);
for (; !messages.empty(); messages.pop())
  stream << messages.front();

Das ja eben nicht... du legst damit die gesamte Anwendung lahm, wenn der Stream nicht schnell genug ist.

[Quiche Lorraine]

@Schlangenmensch

Ich habe das mal als Übung benutzt. Meine Lösung:

#include <fstream>
#include <iostream>
#include <string>
#include <format>

#include <mutex>
#include <thread>
#include <queue>

using namespace std::chrono_literals;

class clogging
{
  private:
	std::queue<std::string> mMsgQueue;
	std::mutex mMutex;
	std::condition_variable mCondition;
	std::atomic_bool mRunning = false;

  public:
	void start()
	{
	    std::unique_lock<std::mutex> lock(mMutex);

	    mRunning = true;
	    while (mRunning)
	    {
	        mCondition.wait(lock, [this] {
	            return !mMsgQueue.empty() || !mRunning;
	        });
			if (!mMsgQueue.empty())
			{
				std::cout << "clogging>> " << mMsgQueue.front() << "\n";
				mMsgQueue.pop();
			}
	    }
	}

	void addmessage(const std::string& msg)
	{
		{
			std::lock_guard<std::mutex> lock(mMutex);

			mMsgQueue.push(msg);			
		}
		mCondition.notify_one();
	}

	void stop()
	{
		mRunning = false;
		mCondition.notify_one();
	}
};


int main()
{
	clogging logging;
	std::jthread t(&clogging::start, &logging);

	for (int i = 0; i < 100; i++)
	{
		std::this_thread::sleep_for(25ms);
		logging.addmessage(std::format("Hallo Welt: {}", i));
		std::this_thread::sleep_for(25ms);
	}
	std::this_thread::sleep_for(10s);
	logging.stop();
	return 0;
}

[Schlangenmensch]

@Quiche-Lorraine Oh, mit std::condition_variable ist schicker, wobei ich ein RAII Lösung bevorzugen würde, damit man das stop nicht vergessen kann.

@Lupus-SLE Wenn du sorge davor hast, dann hol halt immer nur eine Nachricht aus der Queue und log die und nicht alle bisher angelaufenen.

[DocShoe]

Man kann das Ganze ja auch periodisch lösen (code unvollständig, nur zur Information), dann wacht der Logger nur alle 500ms auf und macht die Ausgabe.

#include <string>
#include <thread>
#include <chrono>
#include <vector>

class clogging
{
   std::vector<std::string> messages_;
   std::mutex mutex_;
public:
   clogging();

   void add_message( std::string const& msg )
   {
      std::lock_guard<std::mutex> lock(mutex_);
      messages_.push_back( message );
   }

private:
   void run()
   {
      while( running() )
      {
         std::vector<std::string> messages;
         {
            std::lock_guard<std::mutex> lock(mutex_);
            messages.swap( messages_ );
         }         
         for( auto const& msg : messages )
         {
            std::cout << "clogging>> " << msg << "\n";
         }
         std::this_thread::sleep_for( std::chrono::milliseconds(500) );
      }
   }
};

Noch schöner wird's mit der condition_variable, wie schon von @Quiche-Lorraine gezeigt. Nachteil ist allerdings, dass der Thread träge auf Beendigung reagiert, das kriegt man aber hin, indem man statt 500ms vllt 50x 10ms wartet und dabei prüft, ob der Thread beendet wurde.

[Lupus-SLE]

Btw... bevor die Schlacht weitergeht... geht es eigentlich um Log-Ausgaben, die tatsächlich später noch einmal gelesen, also angefasst werden - oder verschwinden diese einfach in irgendeinem Systemlog? Wenn letzteres der Fall, dann sind so viele Log-Ausgaben doch eigentlich ein Anti-Pattern.

Es gibt doch in Linux diese goldene Regel, "Be quiet except in errors"... und die KI sagt dazu:

Key Aspects of This Principle

• Preventing "Alert Fatigue": By silencing routine success messages, only true errors stand out, reducing the likelihood of missing important failures.
• CI/CD and Automation: In automated scripts or Continuous Integration (CI) steps, a "quiet" command (often achieved via --quiet or similar flags) ensures that a passing job produces no output, making failed jobs immediately obvious.
• Reducing Noise: This principle helps differentiate between expected behavior (warnings/info) and unexpected behavior (errors).
• The "Unless Explicitly Silenced" Exception: While errors should not pass silently, there are rare, deliberate cases where catching a specific exception and doing nothing ("suppressing" it) is intended behavior.

[Schlangenmensch]

@Lupus-SLE Ich würde vermuten, es geht dem TE darum, irgendwas mit Threads zu machen und das Logging ist halt einfach ein Beispiel um damit zu experimentieren.

[Lupus-SLE]

@Schlangenmensch sagte in Klassenfunktion mit Parameter als Thread:

dann hol halt immer nur eine Nachricht aus der Queue und log die und nicht alle bisher angelaufenen

Ich denke auch, das wäre dann am sinnvollsten ... sofern man keinen Zwischencache haben möchte.