Multithreading und GetMessage

Biolunar

Ist doch klar: dein Thread ballert so schnell wie möglich durch die Schleife und hält dabei fast ununterbrochen die CriticalSection. Folglich bleibt fast keine Zeit mehr für die Nachrichtenschleife des Mainthreads; diese wird fast nur am Warten sein.

PS: Du hast mindestens ein Data Race zwischen deinem Thread und dem Mainthread. Der Thread läuft auch noch, wenn das Fenster schon geschlossen wurde, folglich ist das HWND im Thread ungültig. TAbbruch muss außerdem atomic sein.

Martin Richter

Es gibt hier nur offene Threads. Also kannst Du auch einen neuen Anfangen!

Also der Thread sollte nach Ausführung seines Zyklus seine time slice abgeben.

Hier wäre Sleep(1) ein Kandidat. Gegen Sleep(0) spricht http://blogs.msdn.com/oldnewthing/archive/2005/10/04/476847.aspx aber würde auch gehen.

@Biolunar: Eine Race Condition sehe ich nicht. Wo ist die? Beide wollen das gleiche. Und? Dafür gibt es die Critical Section!
Oder verstehst Du was anderes under Race-Condition?
https://de.wikipedia.org/wiki/Race_Condition

Das Problem ist vermutlich einfach in der Priorisierung. Du möchtest das Inputs sofort behandelt werden. Also könntest Du die Thread Prio deines Input Threads erhöhen, und damit käme er öfters dran WENN eine Windows Nachricht ankommt.

Biolunar

Martin Richter schrieb:

@Biolunar: Eine Race Condition sehe ich nicht. Wo ist die?

if (!bitte->Init(hwnd)) { // Hier wird das HWND dem Graphics Objekt übergeben.

// Im Thread:

// Ich vermute mal, dass hier das HWND verwendet wird um das Fenster zu bemalen.
EnterCriticalSection(&window);
bitte->cls(1.0, 1.0, 1.0, 1.0);
bitte->Ball(375.0f, y, 90);
LeaveCriticalSection(&window);

while (GetMessage(&msg, NULL, 0, 0)) 
{ 
    TranslateMessage(&msg); 

    EnterCriticalSection(&window); 
    DispatchMessage(&msg); 
    LeaveCriticalSection(&window); 
} // Wenn das Programm aus diesem Block heraus geht, ist das HWND ungültig, weil das Fenster zerstört wurde.

// Hier kann der Thread noch auf das ungültige HWND zugreifen und tut es vermutlich auch, da die
// CriticalSection im Hauptthread verlassen wurde.

das mit dem ungültigen hwnd stimmt, dachte aber eigentlich die sleep-Funktion würde das lösen... atomic sagt mir nichts, ist das das: https://msdn.microsoft.com/en-us/library/windows/desktop/ff476334(v=vs.85).aspx
oder hättest du ein Beispiel?

time slice sagt mir leider auch nichts und das mit der Priorität probiere ich gleich. und klar, ballert mein thread durch die Schleife in die CriticalSection, aber wenn er das nicht tut, hängt das Bild und das will ich ja auch nicht ...

ganz lieben Dank, freue mich über weitere Hilfe

LRESULT CALLBACK WindowProc(HWND hwnd, UINT uMSG, WPARAM wParam, LPARAM lParam) {

	switch (uMSG)
	{
	case WM_CLOSE:
		if (MessageBox(hwnd, _T("Are you sure?"), _T("noPlan"), MB_OKCANCEL) == IDOK) {
			TAbbruch = true;
			Sleep(5000);
			DestroyWindow(hwnd);
			PostQuitMessage(0);
		}
		return 0;
	}

	return DefWindowProc(hwnd, uMSG, wParam, lParam);
}
[code="cli"]

das ist vermutlich besser für den hwnd

Eine Einstellung der Prioritäten führt leider auch nicht zum Erfolg, ich sehe nur den Ladekringel und kann nichts drücken. Eine andere Idee?

Martin Richter

Kommt keine Nachricht an? Dann ist was anderes faul.
Was war mit einem Sleep(1)?

Bau mal einen Sleep(100) ein im Thread... dann muss ja was passieren.

Th69

Aber natürlich außerhalb der CriticalSection.

Und für Grafikaktualisierung solltest du statt dem Thread besser einen Timer benutzen (der dann auch im UI-Thread läuft).

sleep 100, funktioniert, aber auch mit einem kleinen lag. Außerdem kriege ich jetzt wieder trotz meiner Modifikation einen hwnd-Fehler.

Liegt mein Problem, dass entweder das Bild hängt oder keine Benutzereingaben verarbeitet werden können, an meiner Inkompetenz oder ist es mit multithreading tatsächlich nicht möglich? Kann ich die zu sperrende Ressource in CriticalSection irgendwie spezifizieren, sodass nicht das ganze Fenster gesperrt wird?

@Th69, das Ziel dieses ganzen Vorhabens war eigentlich eine halbwegs vernünftige Übung zu multithreading zu haben...

danke für eure unermüdliche Hilfe!

Warum schreibst du das ganze Zeug nicht in die Callback-Funktion?
Also CreateThread nach WM_CREATE und TAbbruch nach WM_DESTROY

@Wade1234: wahrscheinlich mache ich das falsch, aber bei mir führt das zu einem deadlock...

und bei meiner anderen Frage, bitte keine Scheu...

Danke für eure Hilfe!

Das RaceCondition-Problem ist gelöst und das Programm sollte threadsafe sein, bezüglich der anderen Sache habe ich noch immer keine Idee.

Biolunar

Ich hatte heute mal Lust was neues zu schreiben, daher habe ich ein vollständiges Programm zu deinem Problem geschreiben. Allerdings verwende ich nicht die originale WinAPI, sondern meinen eigenen Header, der sich im Grunde 1-zu-1 zum Microsofts windows.h mappen lässt. Bei mir haben die Windows Dinge Präfixe wie win_ , WIN_ und Win . Der größte Unterschied sind die primitiven Datentypen: so werden bei mir z.B. statt WPARAM uintptr_t und statt INT int32_t verwendet.

Programm ist etwas konfigurierbar durch die Konstanten ganz oben im Code.

#include <libwindows/windows.h>

#include <stdlib.h>
#include <signal.h>
#include <assert.h>
#include <stdio.h>
#include <limits.h>
#include <inttypes.h>
#include <stdatomic.h>
#include <stdbool.h>

// Global Constants
enum
{
	window_width = 640,
	window_height = 480,

	ball_radius = 50,
	vsync = 1,
};
static double const refresh_rate = 60.0; // In Hertz
static double const tick_rate = 1.0 / 100.0; // In seconds

struct State
{
	// Coordinates specify the center of the ball.
	int x;
	int y;
	bool downwards;
};

static atomic_bool g_running = ATOMIC_VAR_INIT(true);

static int64_t get_pfreq(void)
{
	static int64_t freq = 0;
	if (freq == 0)
	{
		union WinLargeInteger li;
		WinBool const ret = win_query_performance_frequency(&li);
		assert(ret != 0);
		freq = li.quad_part;
	}
	return freq;
}

static int64_t get_pcount(void)
{
	union WinLargeInteger li;
	WinBool const ret = win_query_performance_counter(&li);
	assert(ret != 0);
	return li.quad_part;
}

static double get_time(void)
{
	return (double)get_pcount() / (double)get_pfreq();
}

static void signal_handler(int sig)
{
	atomic_store(&g_running, false);
}

static void update(struct State* const state)
{
	assert(state);

	if (state->downwards)
		state->y += 1;
	else
		state->y -= 1;

	if ((state->y <= ball_radius) || (state->y >= window_height - ball_radius))
		state->downwards = !state->downwards;
}

static void render(WinDeviceContext* const dc, struct WinRect const* const rect, struct State const* const state)
{
	assert(dc);
	assert(rect);
	assert(state);

	// Draw background.
	WinBrush* const background = (WinBrush*)win_get_stock_object(win_BLACK_BRUSH);
	win_fill_rect(dc, rect, background);

	// Draw ball.
	win_ellipse(dc, state->x - ball_radius, state->y - ball_radius, state->x + ball_radius, state->y + ball_radius); // Default color is white.
}

static uint32_t WIN_CALLBACK thread_proc(void* const param)
{
	WinWindow* const wnd = param;
	assert(wnd);

	struct WinRect client_rect;
	win_get_client_rect(wnd, &client_rect);

	WinDeviceContext* const dc = win_get_dc(wnd);
	WinDeviceContext* const mem_dc = win_create_compatible_dc(dc);
	WinBitmap* const bitmap = win_create_compatible_bitmap(dc, client_rect.right - client_rect.left, client_rect.bottom - client_rect.top);
	if (!bitmap)
		return 1;
	WinBitmap* const old_bitmap = (WinBitmap*)win_select_object(mem_dc, (WinGdiObj*)bitmap);

	struct State state = { window_width / 2, ball_radius, true }; 

	double seconds_counter = 0.0; // This counter is used to print a message every second.
	uint64_t frames = 0;
	uint64_t updates = 0;

	double previous_time = get_time();
	double time_lag = 0.0;

	if (vsync)
		win_time_begin_period(1);

	while (atomic_load(&g_running))
	{
		while (time_lag >= tick_rate)
		{
			update(&state);
			++updates;

			time_lag -= tick_rate;
		}

		if (seconds_counter >= 1.0)
		{
			printf("FPS: %"PRIu64", Updates: %"PRIu64"\n", frames, updates);

			frames = 0;
			updates = 0;
			seconds_counter -= 1.0;
		}

		render(mem_dc, &client_rect, &state);
		win_bit_blt(dc, client_rect.left, client_rect.top, client_rect.right - client_rect.left, client_rect.bottom - client_rect.top, mem_dc, 0, 0, win_SRCCOPY);
		++frames;

		double const target_seconds = 1.0 / refresh_rate;
		double current_time = get_time();
		double elapsed_time = current_time - previous_time;
		if (vsync && elapsed_time < target_seconds) // Poor mans V-Sync
		{
			// Take a nap.
			uint32_t const sleep_ms = (uint32_t)(1000.0 * (target_seconds - elapsed_time));
			win_sleep(sleep_ms);

			// Spinlock for the rest of the current frame time.
			while ((elapsed_time = current_time - previous_time) < target_seconds)
				current_time = get_time();
		}
		previous_time = current_time;
		time_lag += elapsed_time;

		seconds_counter += elapsed_time;
	}

	win_select_object(mem_dc, (WinGdiObj*)old_bitmap);
	win_delete_object((WinGdiObj*)bitmap);
	win_delete_dc(mem_dc);
	win_release_dc(wnd, dc);
	if (vsync)
		win_time_end_period(1);
	return 0;
}

static intptr_t WIN_CALLBACK wnd_proc(WinWindow* const wnd, uint32_t const msg, uintptr_t const wparam, intptr_t const lparam)
{
	static WinHandle* thread = 0;

	switch (msg)
	{
		case win_WM_CREATE:
		{
			thread = win_create_thread(0, 0, &thread_proc, wnd, 0, 0);
			if (!thread)
				return -1;
			return 0;
		}

		case win_WM_DESTROY:
		{
			atomic_store(&g_running, false);
			win_wait_for_single_object(thread, win_INFINITE);

			win_post_quit_message(EXIT_SUCCESS);
			return 0;
		}
	}

	return win_def_window_proc(wnd, msg, wparam, lparam);
}

int main(void)
{
	signal(SIGINT, &signal_handler);
	signal(SIGTERM, &signal_handler);

	WinInstance* const instance = win_get_module_handle(0);

	struct WinWindowClassEx const wc =
	{
		.size = sizeof wc,
		.instance = instance,
		.window_proc = &wnd_proc,
		.class_name = u"main window",
		.cursor = win_load_cursor(0, WIN_IDC_CROSS),
	};

	WinAtom const atom = win_register_class_ex(&wc);
	if (!atom)
		return EXIT_FAILURE;

	uint32_t const styles = win_WS_BORDER | win_WS_CAPTION | win_WS_SYSMENU;
	uint32_t const ex_styles = win_WS_EX_APPWINDOW;
	struct WinRect window_rect = { 0, 0, window_width, window_height };
	if (!win_adjust_window_rect_ex(&window_rect, styles, win_FALSE, ex_styles))
		return EXIT_FAILURE;

	WinWindow* const wnd = win_create_window_ex(ex_styles, wc.class_name, u"thread test", styles, win_CW_USEDEFAULT, win_CW_USEDEFAULT, window_rect.right - window_rect.left, window_rect.bottom - window_rect.top, 0, 0, instance, 0);
	if (!wnd)
		return EXIT_FAILURE;

	win_show_window(wnd, win_SW_SHOW);

	struct WinMessage msg;
	WinBool ret = 0;
	while ((ret = win_get_message(&msg, 0, 0, 0)) != 0)
	{
		if (ret == -1)
			return EXIT_FAILURE;

		win_translate_message(&msg);
		win_dispatch_message(&msg);
	}

	assert(msg.wparam <= INT_MAX);
	return (int)msg.wparam;
}

AnfängerX schrieb:

@Wade1234: wahrscheinlich mache ich das falsch, aber bei mir führt das zu einem deadlock...

und bei meiner anderen Frage, bitte keine Scheu...

Danke für eure Hilfe!

mir fiel irgendwann heute nacht ein, dass du den thread auch nicht "gejoined" hast.

@Biolunar: Vielen Dank - super cool! ich schaue es mir am Wochenende noch einmal in Ruhe an. Denn die atomic Sachen sind mir neu und die muss ich googeln.

@wade1234: das stimmt, ich wusste nicht, dass das bei einem WindowsThread auch notwendig ist. An dem Problem als solches würde es aber auch nichts ändern oder?

danke für eure Hilfe !

AnfängerX schrieb:

@wade1234: das stimmt, ich wusste nicht, dass das bei einem WindowsThread auch notwendig ist. An dem Problem als solches würde es aber auch nichts ändern oder?

danke für eure Hilfe !

eigentlich müsste sich das problem doch genau dadurch lösen, weil du erst den thread beendest und dann das fenster geschlossen wird.

oder gibt es noch ein anderes problem?