Multithreading loses threads at random

[louwin]

Long story.... I wrote a Mandelbrot program and got it working nicely but it was SLOW so I thought to multithread it.
I divided the screen into 6 slices and ran each slice in a thread. Along the way I came across the fact that SetPixel isn't thread safe etc. So I created alternate device contexts and bitmaps for each thread. Terrific! The speed improvement was phenomenal :)
BUT (there's always a BUT)! I lose "slices" (threads) at random! Sometimes 2 in the same Mandelbrot :(
For instance, I draw 88 different Mandelbrots (should be 528 slices) and I'd lose 10 slices once. 15 slices lost (same Mandelbrots on another run).
I have included the thread code below. Have I not released a bitmap or a DC? Could someone look over my code and see omissions or errors please?

Thread code....

void slice(int minY, int maxY) {
	long double newReal, newImaginary, oldReal, oldImaginary;
	long double pixelX, pixelY;
	int iter, x, y;

	Info.bmiHeader.biHeight = SliceSize;

	COLORREF Colour;//Working colour

	HDC threadDC = CreateCompatibleDC(hdc);

	void *data;
	HBITMAP hBitmap = CreateDIBSection(threadDC, &Info, DIB_RGB_COLORS, (void**)&data, 0, 0);
	SelectObject(threadDC, hBitmap);// Select it into the alternate (threading) DC

	for (y = minY; y < maxY; y++) {
		pixelY = (y - halfheight) / (zoom * halfheight) + moveY;
		for (x = 0; x < width; x++) {
			pixelX = 1.5 * (x - halfwidth) / (zoom * halfwidth) + moveX;
			newReal = newImaginary = 0; //these should start at zero  Z = Z * Z + C
			for (iter = 0; iter < maxIterations; iter++) {
				oldReal = newReal; oldImaginary = newImaginary;
				newReal = oldReal * oldReal - oldImaginary * oldImaginary + pixelX;
				newImaginary = 2 * oldReal * oldImaginary + pixelY;
				if ((newReal * newReal + newImaginary * newImaginary) > 4) {
				     break;
				}
			}
			if (iter < maxIterations) { 
				Colour = Colours[iter % 306];
			} else {
				Colour = BLACK;
			}
			SetPixel(threadDC, x, y % SliceSize, Colour);
		}//End of for(x (width loop).
		MSG msg;
		if (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
			TranslateMessage(&msg);
			DispatchMessage(&msg);
		}
	}//End of for(y (height loop)
	BitBlt(hdc, 0, minY, width, maxY, threadDC, 0, 0, SRCCOPY);
	DeleteObject(hBitmap);
	DeleteDC(threadDC);
}

Driver code

       std::thread threads[number_of_threads];
	
	for (int i = 0; i < number_of_threads; ++i) {
		threads[i] = thread(slice, i * SliceSize, (i + 1) * SliceSize);
	}
	
	for (int i = 0; i < number_of_threads; ++i) {
		threads[i].join();
	}

[raschupkin]

Problem is in the line: BitBlt(hdc, 0, minY, width, maxY, threadDC, 0, 0, SRCCOPY);
global hdc is updated simultaneously from different threads. Two threads can read the same version of hdc, each update own copy, then write changes. So it can happen that changes of only one thread will finally be written. You need to add locking with a mutex, as described at https://msdn.microsoft.com/en-us/library/windows/desktop/ms686927(v=vs.85).aspx
I don't remember Windows mutexes well, but it should be like:
global

HANDLE ghMutex; 
...
ghMutex = CreateMutex( 
        NULL,              // default security attributes
        FALSE,             // initially not owned
        NULL);             // unnamed mutex
...
CloseHandle(ghMutex);

In thread:

        dwWaitResult = WaitForSingleObject( 
            ghMutex,    // handle to mutex
            INFINITE);  // no time-out interval
        BitBlt(hdc, 0, minY, width, maxY, threadDC, 0, 0, SRCCOPY);
        ReleaseMutex(ghMutex);

Or if you use std::thread:
global

std::mutex m;

In thread:

        m.lock();
        BitBlt(hdc, 0, minY, width, maxY, threadDC, 0, 0, SRCCOPY);
        m.unlock();

[louwin]

Thanks raschupkin. :)

What a PERFECT response! :)

I incorporated your code and reran the 88 Mandelbrots. NOT one "lost" "slice". :)

THANK YOU :) :) :)