Calculating prime numbers using multi threading

Question

I started to work with threads recently and I tried to run a simple program that uses threads but I get really strange output. The program writes the prime numbers in the given range with N(parameter to the function)number of threads into the file "PRIMES.txt", if the range <= 1000 the output is fine but if the range is bigger, then the output is something like :

‰‱′″‵‷ㄱㄠ″㜱ㄠ‹㌲㈠‹ㄳ㌠‷ㄴ㐠″㜴㔠″㤵㘠‱㜶㜠‱㌷㜠‹㌸㠠‹㜹ㄠ㄰ㄠ㌰ㄠ㜰ㄠ㤰ㄠ㌱ㄠ㜲ㄠㄳㄠ㜳ㄠ㤳ㄠ㤴ㄠㄵㄠ㜵ㄠ㌶ㄠ㜶ㄠ㌷ㄠ㤷... (much longer)

What would be the problem?

Here is my code :

threads.h :

#include <fstream>
#include <string>
#include <iostream>
#include <thread>
using namespace std;

void writePrimesToFile(int begin, int end, ofstream& file);
void callWritePrimesMultipleThreads(int begin, int end, string filePath, int N);

threads.cpp :

#include "Threads.h"
#include <iostream>
#include <string>
#include <fstream>
#include <thread>
#include <mutex>
#include <vector>

mutex mtx;

void PrimesToFile(int begin, int end, ofstream& file)
{
bool isPrime;
string Primes;
int count = 0;
mtx.lock();
cout << "Thread is running" << endl;
    for (int i = begin; i < end; i++)
    {
        isPrime = true;
        for (int j = 2; j < i; j++)
        {
            if (i%j == 0)
                isPrime = false;
        }
        if (isPrime)
        {
            Primes.append(to_string(i));
            Primes.append(" ");
        }
    }
file.write(Primes.c_str(), Primes.length());
mtx.unlock();
}

void WritePrimesMultipleThreads(int begin, int end, string filePath, int N)
{
    ofstream OP;
    OP.open(filePath);
    int lastPos = 0;
    int destPos = end / N;
    thread* TV = new thread[N];
    for (int i = 0; i < N; i++)
    {
        TV[i] = thread(PrimesToFile, lastPos, destPos, ref(OP));
        lastPos = destPos;
        destPos += end / N;
    }
    for (int i = 0; i < N; i++)
        TV[i].join();
}

Starting point :

#include "Threads.h"
#include <iostream>
#include <string>
#include <fstream>
#include <thread>

void main()
{
    WritePrimesMultipleThreads(1, 10000, "PRIMES.txt", 5);
    system("PAUSE");
}

Thanks!

I think you're creating too many threads. Try to fix them at certain level. Why would you need 1000 threads? This is not solution for the problem, but I think this worth considering. [How many threads is too many?](http://stackoverflow.com/questions/481970/how-many-threads-is-too-many) Note that question I'm referring to is about servers. Also you don't delete them. — Incomputable, Jan 31 '16 at 16:46
I created only 5 as mentioned in the call WritePrimesMultipleThreads(1, 10000, "PRIMES.txt", 5); and when I try to delete them it throws an exception. — Sunz, Jan 31 '16 at 16:47
Your program is effectively singlethreaded since the mutex is locked throughout the function. — molbdnilo, Jan 31 '16 at 17:07
changing stream to std::strstream solved the problem partially, but if code will be corrected as @molbdnilo says, your primes won't be sorted, something like 79 83 89 6007 8009 4001 (from my output). Despite that, rubbish at the end still exists. — Incomputable, Jan 31 '16 at 17:27

score 1 · Answer 1 · edited May 23 '17 at 11:54

Hours of debugging turned out to be in wrong implementation of std::ofstream. Just outputting at the beginning OP << "\n" solved the problem. Compiler is MSVC 2015 update 1. More about about it here. Additionally, you have resource leak, single threading when it is not really intended to, not efficient algorithm of finding primes in a range, compiling errors in your posted code, unnecessary writePrimesToFile function and header files in your header file, you're using using namespace std and may be more problems. I recommend you posting your code at codereview.stackexchange.com to make the code better, because solving this problem is not enough to solve the original problem. EDIT: You need to flush the stream every time you done writing something.

Calculating prime numbers using multi threading

1 Answers1