Mandelbrot: Improve thread handling code.

* Application is now much more stable and less likely to crash
 * Almost all communication to render threads is done through semaphores.

Change-Id: I7858cdb4a2b83ef4bfe44588cebef01bd3304346
Reviewed-on: https://review.haiku-os.org/730
Reviewed-by: waddlesplash <[email protected]>

src/apps/mandelbrot/FractalEngine.cpp

@@ -33,13 +33,13 @@ FractalEngine::FractalEngine(BHandler* parent, BLooper* looper)
 	fRenderBuffer(NULL),
 	fRenderBufferLen(0),
 	fMessenger(parent, looper),
-	fThreadsRendering(0),
 	fIterations(1024),
 	fColorset(Colorset_Royal)
 {
 	fDoSet = &FractalEngine::DoSet_Mandelbrot;
 
 	fRenderSem = create_sem(0, "RenderSem");
+	fRenderStoppedSem = create_sem(0,"RenderStopped");
 
 	system_info info;
 	get_system_info(&info);
@@ -49,8 +49,6 @@ FractalEngine::FractalEngine(BHandler* parent, BLooper* looper)
 		fThreadCount = MAX_RENDER_THREADS;
 
 	for (uint8 i = 0; i < fThreadCount; i++) {
-		fRestartRenderThread[i] = false;
-		fRenderThreadRunning[i] = false;
 		fRenderThreads[i] = spawn_thread(&FractalEngine::RenderThread,
 			BString().SetToFormat("RenderThread%d", i).String(),
 			B_NORMAL_PRIORITY, this);
@@ -95,6 +93,14 @@ void FractalEngine::MessageReceived(BMessage* msg)
 		break;
 
 	case MSG_RESIZE: {
+		TRACE("Got MSG_RESIZE threads rendering\n");
+		if (fStopRender) {
+			// Will be true throughout this whole handler. Set false at the end
+			TRACE("Breaking out of MSG_RESIZE handler\n");
+			break;
+		}
+		StopRender();
+
 		delete fRenderBuffer;
 		fRenderBuffer = NULL;
 
@@ -102,17 +108,20 @@ void FractalEngine::MessageReceived(BMessage* msg)
 		fHeight = msg->GetUInt16("height", 240);
 
 		TRACE("Creating new buffer. width %u height %u\n", fWidth, fHeight);
-
 		fRenderBufferLen = fWidth * fHeight * 3;
 		fRenderBuffer = new uint8[fRenderBufferLen];
 		memset(fRenderBuffer, 0, fRenderBufferLen);
 
 		BMessage message(MSG_BUFFER_CREATED);
 		fMessenger.SendMessage(&message);
+
+		fStopRender = false;
 		break;
 	}
 	case MSG_RENDER: {
-		TRACE("Got MSG_RENDER. fThreadsRendering = %u\n",fThreadsRendering);
+		TRACE("Got MSG_RENDER.\n");
+		StopRender();
+		fStopRender = false;
 		Render(msg->GetDouble("locationX", 0), msg->GetDouble("locationY", 0),
 			msg->GetDouble("size", 0.005));
 		break;
@@ -121,9 +130,9 @@ void FractalEngine::MessageReceived(BMessage* msg)
 		TRACE("Got MSG_THREAD_RENDER_COMPLETE for thread %d\n",
 			msg->GetUInt8("thread", 0));
 
-		fThreadsRendering--;
+		int32 threadsStopped;
-		TRACE("Set fRendering = %d\n", fThreadsRendering);
+		get_sem_count(fRenderStoppedSem, &threadsStopped);
-		if (!fThreadsRendering) {
+		if (threadsStopped == fThreadCount) {
 			TRACE("Done rendering!\n");
 			BMessage message(MSG_RENDER_COMPLETE);
 			fMessenger.SendMessage(&message);
@@ -145,9 +154,32 @@ void FractalEngine::WriteToBitmap(BBitmap* bitmap)
 		B_RGB24);
 }
 
+void FractalEngine::StopRender()
+{
+	if (fRenderStopped)
+		return;
+	fRenderStopped = true;
+		// true if another call to StopRender() won't work properly because
+		// the fRenderStoppedSem semaphores have already been acquired.
+	TRACE("Stopping render...\n");
+	fStopRender = true;
+	for (uint i = 0; i<fThreadCount; i++)
+		acquire_sem(fRenderStoppedSem);
+
+	TRACE("Render stopped.\n");
+		// note that fStopRender is NOT set to false at the end here.
+		// This is to allow the message handlers to use this variable to
+		// block duplication of stuff.
+}
+
 
 void FractalEngine::Render(double locationX, double locationY, double size)
 {
+	fRenderStopped = false;
+		// This means that future Render calls will need to call stop render
+	if (fStopRender)
+		debugger("Error: Render shouldn't be called while fStopRender = true\n");
+
 	fLocationX = locationX;
 	fLocationY = locationY;
 	fSize = size;
@@ -155,18 +187,10 @@ void FractalEngine::Render(double locationX, double locationY, double size)
 	TRACE("Location: %g;%g;%g\n", fLocationX, fLocationY, fSize);
 
 	for (uint8 i = 0; i < fThreadCount; i++) {
-		if (fRenderThreadRunning[i])
+		release_sem(fRenderSem);
-			fRestartRenderThread[i] = true;
-		else
-			release_sem(fRenderSem);
-		fRenderThreadRunning[i] = true;
 	}
-
-	fThreadsRendering = fThreadCount;
-	TRACE("Set fRendering = %d\n", fThreadsRendering);
 }
 
-
 status_t FractalEngine::RenderThread(void* data)
 {
 	FractalEngine* engine = static_cast<FractalEngine*>(data);
@@ -179,19 +203,12 @@ status_t FractalEngine::RenderThread(void* data)
 		}
 	}
 
-	bool restarting = false;
 	while (true) {
-		TRACE("Thread %d starting another render pass\n", threadNum);
+		release_sem(engine->fRenderStoppedSem);
-
-		if (!restarting) {
-			TRACE("Thread %d awaiting semaphore...\n", threadNum);
-			acquire_sem(engine->fRenderSem);
-			TRACE("Thread %d got semaphore!\n", threadNum);
-		} else {
-			TRACE("Thread %d setting restart = false\n", threadNum);
-			restarting = false;
-		}
 
+		TRACE("Thread %d awaiting semaphore...\n", threadNum);
+		acquire_sem(engine->fRenderSem);
+		TRACE("Thread %d got semaphore!\n", threadNum);
 
 		uint16 width = engine->fWidth;
 		uint16 height = engine->fHeight;
@@ -213,17 +230,13 @@ status_t FractalEngine::RenderThread(void* data)
 					// halfHeight-(halfHeight-1)-1 = 0
 			}
 
-			if (engine->fRestartRenderThread[threadNum]) {
+			if (engine->fStopRender) {
-				TRACE("Thread %d Got restart message\n", threadNum);
+				TRACE("Thread %d stopping\n", threadNum);
-
-				engine->fRestartRenderThread[threadNum] = false;
-				restarting = true;
 				break; // Restart the loop to update width, height, etc.
 			}
 		}
 
-		if (!restarting) {
+		if (!engine->fStopRender) {
-			engine->fRenderThreadRunning[threadNum] = false;
 			BMessage message(FractalEngine::MSG_THREAD_RENDER_COMPLETE);
 			message.AddUInt8("thread", threadNum);
 			engine->PostMessage(&message);

src/apps/mandelbrot/FractalEngine.h

@@ -50,10 +50,10 @@ private:
 	uint8 fThreadCount;
 	thread_id fRenderThreads[MAX_RENDER_THREADS];
 	sem_id fRenderSem;
+	sem_id fRenderStoppedSem;
 
-	uint8 fThreadsRendering;
+	bool fStopRender;
-	bool fRestartRenderThread[MAX_RENDER_THREADS];
+	bool fRenderStopped;
-	bool fRenderThreadRunning[MAX_RENDER_THREADS];
 
 	double fLocationX;
 	double fLocationY;
@@ -66,6 +66,7 @@ private:
 	int32 (FractalEngine::*fDoSet)(double real, double imaginary);
 
 	void Render(double locationX, double locationY, double size);
+	void StopRender();
 	static status_t RenderThread(void* data);
 	void RenderPixel(uint32 x, uint32 y);