Thread: std::thread leads to Abort

  1. #1
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587

    std::thread leads to Abort

    I'm trying to code a program for dealing with prime numbers. The idea is to have the prime list readable by all threads, but writeable by 1.

    Code:
    #include "gmpxx.h"
    
    #include <iostream>
    #include <thread>
    #include <vector>
    
    // primes must be initialised with at least the first two prime numbers.
    static std::vector<mpz_class> primes = {2, 3};
    
    // integer sqrt
    static mpz_class sqrt(mpz_class n)
    {
        mpf_class x, y = n;
        
        do{
            x = y;
            y = (x + n / x) / 2;
        }while(abs(y - x) >= 1);
        
        return y;
    }
    
    // extends primes to the lowest prime greater than or equal to n
    static void extend_primes(mpz_class n)
    {
        // Unfortunately, the primes can't efficiently, if at all, be extended concurrently...
        static std::mutex m;
        std::lock_guard<std::mutex> lock(m);
        
        for(mpz_class i = primes.back() + 2;i <= n;i += 2)
        {
            bool prime = true; // Assume i is prime.
            
            for(auto j = primes.begin();prime && *j <= sqrt(i);j++)
                if(i % *j == 0)
                    prime = false;
            
            if(prime)
                primes.push_back(i);
        }
    }
    
    int main()
    {
        extend_primes(7);
        std::thread t(extend_primes, 19); // This (or a resulting call) must be the cause. When commented out, everything works fine.
        
        for(auto i = primes.begin();i < primes.end();i++)
            std::cout << i->get_str() << " ";
        
        std::cout << std::endl;
        return 0;
    }
    The error:
    Code:
    $ gcc -ggdb -std=c++0x -lstdc++ -lgmp -o prime prime.cpp
    $ ./prime 
    terminate called after throwing an instance of 'std::system_error'
      what():  
    Aborted
    Last edited by User Name:; 06-02-2011 at 12:11 AM.

  2. #2
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    You start a thread to calculate the primes up to 19, but then immediately go and try outputting the list, whilst the other thread is generating them, and without any locking. Yes I can see why it causes problems, epecially when the vector resizes and all the iterators are invalidated. I assume you understand that the point of a thread is that it doesn't block execution of the thread that created it.
    The thing that reads from the list needs to lock also.

    Other problems...
    You hold the lock whilst generating the entire list of primes. Surely you want a finer grained lock, i.e. only locking as you actually do a push_back? However, once you do that, you wont be able to iterate through the vector using iterators at the same time. I suggest either using a std::list instead, or simply iterating by index. Then you can simultaneously generate primes in one thread and stream them out from other threads.
    You're calculating the square root every time through the loop.
    I would expect that using mpz_sqrt would at least be faster than hand-rolling a basic iteration method, so why not just use that?

    I would like to commend you on your nice use of modern C++
    Last edited by iMalc; 06-02-2011 at 03:53 AM.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  3. #3
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    Quote Originally Posted by iMalc View Post
    You start a thread to calculate the primes up to 19, but then immediately go and try outputting the list, whilst the other thread is generating them, and without any locking. Yes I can see why it causes problems, epecially when the vector resizes and all the iterators are invalidated. I assume you understand that the point of a thread is that it doesn't block execution of the thread that created it.
    The thing that reads from the list needs to lock also.
    Okay, so I need a write lock that will lock all write and read locks. I will work on that, but I think I'm still not understanding why it's aborting with the following modification:
    Code:
    static std::mutex m;
    
    static void extend_primes(mpz_class n)
    {
    	// Unfortunately, the primes can't effeciently, if at all, be extended concurrently...
    	std::lock_guard<std::mutex> lock(m);
    	
    	for(mpz_class i = primes.back() + 2;i <= n;i += 2)
    	{
    		bool prime = true; // Assume i is prime.
    		
    		for(auto j = primes.begin();prime && *j <= sqrt(i);j++)
    			if(i % *j == 0)
    				prime = false;
    		
    		if(prime)
    			primes.push_back(i);
    	}
    }
    
    int main()
    {
    	std::thread t(extend_primes, 19);
    	
    	std::lock_guard<std::mutex> lock(m); // Wait to print
    	
    	for(auto i = primes.begin();i < primes.end();i++)
    		std::cout << i->get_str() << " ";
    	
    	std::cout << std::endl;
    	return 0;
    }
    Quote Originally Posted by iMalc View Post
    You hold the lock whilst generating the entire list of primes. Surely you want a finer grained lock, i.e. only locking as you actually do a push_back? However, once you do that, you wont be able to iterate through the vector using iterators at the same time. I suggest either using a std::list instead, or simply iterating by index. Then you can simultaneously generate primes in one thread and stream them out from other threads.
    But, if I do that, I can't make the assumption that primes contains all primes from 2 to primes.back(), and from that, it follows that I cannot be certain I have the primes necessary to check if primes.back() + 2 is prime, from which it follows that I cannot generate new primes without resorting to brute force checking. I need to be certain of these properties to be able to use my previously generated primes to do an optimal primality check. I need the lock to make sure I insert, in order, all primes from primes.back() to n. I chose not to use a std::set (for its ordered-ness) because it uses extra complexity that I can avoid by inserting them in order. I get what you mean, it's just that there is a good reason for the coarse grained control I'm using.

    Quote Originally Posted by iMalc View Post
    You're calculating the square root every time through the loop.
    I would expect that using mpz_sqrt would at least be faster than hand-rolling a basic iteration method, so why not just use that?
    I didn't know they had an integer square root. Thanks. I was also assuming GCC would optimise away unnecessary sqrt() calls. I will profile it when I'm done, with -03 enabled, and if it dosen't get rid of them, I'll create a temporary and so on.

    Quote Originally Posted by iMalc View Post
    I would like to commend you on your nice use of modern C++
    Thanks. I'm so excited about all the new stuff. I've started like 5 new projects in the last few weeks (since the final draft) just to play with regex, thread, etc.

  4. #4
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I'm only going to say say thing now...
    What you are doing is wrong, absolutely wrong.
    A lock is meant to cause exclusive access to certain resources. This is clearly not what you are trying to do.
    What you need is an event. The thread should notify the main function that it has completed, or the main thread can wait until the thread is complete.
    What you are doing ends up in a race condition. That is, the code may have two outcomes depending on which thread acquires the lock first.

    I'd suggest the later of the two since it's portable. There should be a join member that will block until the thread is finished. Use it.
    Unfortunately, to my disappointment, I have not found an easy and portable way of using events. It's a shame, really. Windows supports it gracefully, yet boost neglects to add it.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  5. #5
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    Sorry I failed to mention this earlier, but join() doesn't work either. It was actually my first try. [In fact, that is why std::mutex m was originally a static local of extend_primes(); it was intended as a mutually exclusive entrance guard for extend_primes(). My later modification was rushed, and admittedly ill conceived.]

    Code:
    int main()
    {
    	std::thread t(extend_primes, 19);
    	t.join();
    	
    	for(auto i = primes.begin();i < primes.end();i++)
    		std::cout << i->get_str() << " ";
    	
    	std::cout << std::endl;
    	return 0;
    }
    Same error.

  6. #6
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    How does your mpz_class look like?
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  7. #7
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    Um... I'm confused. I didn't code it; it's GMP's.

  8. #8
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I see... let's just cut away that part and go with ints then...
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  9. #9
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    Code:
    #include "gmpxx.h"
    
    #include <cmath>
    #include <iostream>
    #include <set>
    #include <thread>
    #include <vector>
    
    // primes must be initialized with at least the first two prime numbers.
    static std::vector<int> primes = {2, 3};
    
    static void extend_primes(int n)
    {
    	// Unfortunately, the primes can't effeciently, if at all, be extended concurrently...
    	static std::mutex m;
    	std::lock_guard<std::mutex> lock(m);
    	
    	for(int i = primes.back() + 2;i <= n;i += 2)
    	{
    		bool prime = true; // Assume i is prime.
    		
    		for(auto j = primes.begin();prime && *j <= sqrt(i);j++)
    			if(i % *j == 0)
    				prime = false;
    		
    		if(prime)
    			primes.push_back(i);
    	}
    }
    
    int main()
    {
    	std::thread t(extend_primes, 19);
    	t.join();
    	
    	for(auto i = primes.begin();i < primes.end();i++)
    		std::cout << *i << " ";
    	
    	std::cout << std::endl;
    	return 0;
    }
    I also cut out my sqrt, since, with ints, it is unnecessary.

    Still same error, though.

  10. #10
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I still can't reproduce your problem.
    My VS10 compliant code if anyone else finds they need it:
    Code:
    #include <cmath>
    #include <iostream>
    #include <set>
    #include <boost/thread.hpp>
    #include <vector>
    #include <cmath>
    
    // primes must be initialized with at least the first two prime numbers.
    static std::vector<int> primes;
    
    static void extend_primes(int n)
    {
    	// Unfortunately, the primes can't effeciently, if at all, be extended concurrently...
    	static boost::mutex m;
    	boost::lock_guard<boost::mutex> lock(m);
    	
    	for(int i = primes.back() + 2;i <= n;i += 2)
    	{
    		bool prime = true; // Assume i is prime.
    		
    		for(auto j = primes.begin();prime && *j <= (int)std::sqrt((double)i);j++)
    			if(i % *j == 0)
    				prime = false;
    		
    		if(prime)
    			primes.push_back(i);
    	}
    }
    
    int main()
    {
    	primes.push_back(2);
    	primes.push_back(3);
    
    	boost::thread t(extend_primes, 19);
    	t.join();
    	
    	for(auto i = primes.begin();i < primes.end();i++)
    		std::cout << *i << " ";
    	
    	std::cout << std::endl;
    	return 0;
    }
    You need to bring out your debugger.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  11. #11
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    It's GCC 4.5.2's thread implementation, I think.

    I made a SMALL test case, and get the same error.

    Code:
    #include <thread>
    
    using namespace std;
    
    void test() {}
    
    int main()
    {
    	thread t(test);
    }
    Lesson learned: Don't assume standard libraries work like they're supposed to.

  12. #12
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    That's a pretty big error... is this some alpha release or something?
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  13. #13
    Password:
    Join Date
    Dec 2009
    Location
    NC
    Posts
    587
    No, far from it, it ships default with Ubuntu.

  14. #14
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    I find it very difficult to believe they would just throw in something that is half-finished or doesn't work properly into something such as a distribution...
    Well, I dunno. You might try boost's threading facilities until the "bugs" are fixed.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  15. #15
    Registered User
    Join Date
    Dec 2006
    Location
    Canada
    Posts
    3,229
    I can't believe they STILL haven't fixed this.

    You need to add "-pthread".

    std::thread segfault

    2 years ago!!

    A more descriptive error message would be nice...

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Weird typo leads to interesting result... what does it mean?
    By pollypocket4eva in forum C++ Programming
    Replies: 4
    Last Post: 06-17-2010, 03:30 AM
  2. abort() causes a segfault
    By JFonseka in forum C Programming
    Replies: 12
    Last Post: 04-16-2008, 01:40 AM
  3. abort()
    By swgh in forum C++ Programming
    Replies: 5
    Last Post: 02-22-2008, 04:11 AM
  4. Abort Procedure
    By incognito in forum Windows Programming
    Replies: 5
    Last Post: 01-02-2004, 09:49 PM
  5. Western society leads to unsatisfaction
    By Zewu in forum A Brief History of Cprogramming.com
    Replies: 20
    Last Post: 08-06-2003, 03:45 PM