1

I'm new for threads and mutex and ı trying to learn them. I write some code that generally create a queue that enqueue all of the numbers from the file ( this file has nearly 20.000 lines and lots of information) since this file contains a lots of information for procees ı need to have multithreads, at the beginning user create the number of threads, then ı stuck at the part that in a while loop ı wanted see which threads enter the loop for dequing the id from queue, but apparently just the first created thread enters and dequeue all of them, ı used mutex for ensure that while a thread enters the loop make it process (dequeue a number) then unlock this mutex in order to other threads can enter but apprently ı did a mistake . Here is the code `

void printer( DynIntQueue  & myQueue) // takes the Dynamic Queue 
{
    //int count = 0;
    queMutex.lock(); // lock it before any threads come it 
    while(!myQueue.isEmpty()) // check this condition
    {
        int num;
        
        cout << "Thread " << this_thread::get_id << " is working" << endl; // this is for printing out which threads enter this loop
        myQueue.dequeue(num); // deqeueu this number from queue
        
        queMutex.unlock(); // unlock this in order to next thread might enter
        queMutex.lock();   // when this thread enters lock it in order to other threads to wait
        
        
    }
    queMutex.unlock(); // if myQueue is empty since it is firsly locked, unlock this 
    
    
}`

My output is like this: Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working Thread 005C1659 is working This goes on until the myQueue is empty with the same thread. What can ı do ensure that other threads might enter this loop?

Edited: Here is the main part `

int main()
{
    DynIntQueue firstQueue;
    
    ifstream input;
    string line;
    int numofthreads;

    input.open("data.tsv");
    getline(input, line); // for first empty
    int id, houseAge, avgRooms, avgBedRooms, latitue, longitute, medianPrice;

    cout << "Please enter the number of threads you want " << endl;
    cin >> numofthreads;
    
    vector <thread> Threads(numofthreads);

    while (!input.eof())
    {
        getline(input, line);
        istringstream divider(line);
        divider >> id >> houseAge >> avgRooms >> avgBedRooms >> latitue >> longitute >> medianPrice;
        firstQueue.enqueue(id);
    }

    for (int i = 0; i < numofthreads; i++)
    {
        Threads[i] = thread(&printer, ref(firstQueue));

    }   
    
     for (int i = 0; i < numofthreads; i++)
     {
         Threads[i].join();
     }
     
     return 0;
}
Bill Lynch
  • 72,481
  • 14
  • 116
  • 162
Razbolt
  • 37
  • 6
  • The problem might be that there is no time interval between unlocking and locking. You can put the current thread to sleep for some time so simulate some work done there. – Daniel Langr Aug 16 '20 at 16:02
  • Also, you don't show us how threads are created. Please, post a [complete minimal compilable example code](https://stackoverflow.com/help/minimal-reproducible-example). – Daniel Langr Aug 16 '20 at 16:03
  • https://stackoverflow.com/a/17528655/362589 – Dani Aug 16 '20 at 16:08
  • Unlocking a mutex does not give a different waiting thread "first dibs" on it. – molbdnilo Aug 16 '20 at 16:10
  • The logic is faulty. That's not how one performs such run. You ought to have another variable that tells whether the run is finished and utilize `std::condition_variable` to wait for input while the queue is empty. Also all locking/unlocking should be managed by RAII locks like `std::unique_lock`. – ALX23z Aug 16 '20 at 16:10
  • 1
    This is a questionable line of reasoning: "since this file contains a lots of information for procees ı need to have multithreads". That conclusion does not follow from the premise. – molbdnilo Aug 16 '20 at 16:12
  • 1
    Mandatory read: [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – molbdnilo Aug 16 '20 at 16:13
  • 1
    Possibly also: [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/q/1452721/580083). – Daniel Langr Aug 16 '20 at 16:14

1 Answers1

3

Note: std::this_thread::get_id() is a function, so you should be calling it. I assume this is just a copy/paste error.

If I add some work between opening and closing the queue, i clearly see two threads using the queue.

I don't think you have any issue with the code shown.

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


struct DynIntQueue {
  bool isEmpty() const { return q_.empty(); }
  void dequeue(int &elem) { elem = q_.front(); q_.pop(); }
  std::queue<int> q_{{10, 20, 4, 8, 92}};
};

std::mutex queMutex;

void printer( DynIntQueue  & myQueue) {
    queMutex.lock();
    while(!myQueue.isEmpty()) {
      int num;
      std::cout << "Thread " << std::this_thread::get_id() << " is working" << std::endl;
      myQueue.dequeue(num);
      queMutex.unlock();
      std::cout << "working outside the lock" << std::endl;
      std::cout << "working outside the lock" << std::endl;
      std::cout << "working outside the lock" << std::endl;
      std::cout << "working outside the lock" << std::endl;
      std::cout << "working outside the lock" << std::endl;
      queMutex.lock();
    }
    queMutex.unlock();   
}

int main() {
  std::cout << "Hello World!\n";
  DynIntQueue q;

  std::thread t1([&q]() { printer(q); });
  std::thread t2([&q]() { printer(q); });
  t1.join();
  t2.join();
}
$ clang++-7 -pthread -std=c++17 -o main main.c
$ ./main
Hello World!
Thread 139686844172032 is working
working outside the lock
working outside the lock
working outside the lock
working outside the lock
working outside the lock
Thread 139686835779328 is working
working outside the lock
working outside the lock
working outside the lock
working outside the lock
working outside the lock
Thread 139686844172032 is working
working outside the lock
working outside the lock
working outside the lock
working outside the lock
working outside the lock
Thread 139686835779328 is working
working outside the lock
working outside the lock
working outside the lock
working outside the lock
working outside the lock
Thread 139686844172032 is working
working outside the lock
working outside the lock
working outside the lock
working outside the lock
working outside the lock
Bill Lynch
  • 72,481
  • 14
  • 116
  • 162
  • I used sleep for 1 second method between unlock and lock ı still have problem here, ı also edit again with the main part . – Razbolt Aug 16 '20 at 16:18
  • @Razbolt: Did you specify numOfThreads as 1? Could you rewrite your main such that it doesn't require user input (I'd suggest hard-coding it like I did in this answer). – Bill Lynch Aug 16 '20 at 16:19
  • 2
    Also, I was assuming this was just a copy paste error, but `std::this_thread::get_id()` is a function, so you should be calling it. – Bill Lynch Aug 16 '20 at 16:21
  • @BillLynch Good catch, it seems to be the problem, since the output number is in hex is OP's case, which is by default for pointers. OP likely prints out the address of `get_id` function. You should put it into the answer. – Daniel Langr Aug 16 '20 at 16:26
  • 1
    Thanks a lot, this was a get_id() problem, it makes me sick that ı m struggle with this issue for 2 days :D – Razbolt Aug 16 '20 at 16:36
  • @Razbolt: Consider turning on more warning in your compiler. Given that error, clang++7 would fail with warn with `main.cpp:22:33: warning: address of function 'std::this_thread::get_id' will always evaluate to 'true' [-Wpointer-bool-conversion]` – Bill Lynch Aug 16 '20 at 17:06