CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 2 of 2 FirstFirst 12
Results 16 to 21 of 21
  1. #16
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,765

    Re: Segmentation Fault ??

    I have not taken a really good look at your code, but just a warning: the CSocket destructor should be declared virtual. Declaring the destructor of its derived class as virtual is not enough.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  2. #17
    John E is offline Elite Member Power Poster
    Join Date
    Apr 2001
    Location
    Manchester, England
    Posts
    4,835

    Re: Segmentation Fault ??

    One other thing.... how does your program know that the object will need to be deleted in precisely 4 minutes' time? Generally speaking, you delete an object when you no longer need it and you don't normally know this, four minutes in advance.

    The 4 minute delay suggests to me that some other process is somehow still using the object and you're just making a guess about how long it might need it for.

    Or to put it another way, try re-instating the virtual function from yesterday but increase 240 to 360. My guess is that this will also make the problem magically go away. So will a lot of other arbitrary changes. You need to look for the kind of thing that Laserlight suggested. It'll end up being something like that or a pointer corruption problem.

    It's a great shame that you can't reproduce the problem in VC++ because it's much better for debugging this kind of problem than gdb.
    "A problem well stated is a problem half solved.” - Charles F. Kettering

  3. #18
    Join Date
    Jul 2002
    Location
    Portsmouth. United Kingdom
    Posts
    2,727

    Re: Segmentation Fault ??

    m_ClientsDeleting.push_back(client);

    i = m_ClientsDeleting.erase( i );

    Are these two function calls occurring in the same thread?

    If they are not, then this will almost certainly be a source of memory corruption.
    "It doesn't matter how beautiful your theory is, it doesn't matter how smart you are. If it doesn't agree with experiment, it's wrong."
    Richard P. Feynman

  4. #19
    Join Date
    Sep 2011
    Posts
    13

    Re: Segmentation Fault ??

    yes, i do not insert elements into vectors or containers of alien threads.
    the Clients* instance is used for the MYSQL login process of the clients.
    .erase( ) is called on the same thread. all i share with other threads is boolean variables.

    so the only thread that tries to access it is the mysql thread with the mysql login function.
    although the function is safe because

    i get the std::string m_Name of the client and copy it so i can use it in my queries.
    then i try to access the pointer at the end of the query so i can set the :

    Clients::m_Identified flag to true.

    ii use the CBoolExpr( ) function for that.meaning that the object is used at the start of the function before the queries (thus the blocking of code)

    and at the end of the function after the queries.


    Quote Originally Posted by laserlight View Post
    I have not taken a really good look at your code, but just a warning: the CSocket destructor should be declared virtual. Declaring the destructor of its derived class as virtual is not enough.
    thanks, didn't notice that before.

    ---EDIT


    so the server is still running from yesterday, but i'm 99% sure you guys are right about memory corruption in my code.
    although i don't precisely know why.

    here's the destructor of the Clients class

    Code:
    Clients :: ~Clients( )
    {
    	delete m_Socket;
    
    	m_Socket = NULL;
    
    	while(!m_CPackets.empty() )
    	{
    		delete m_CPackets.front();
    		m_Packets.pop();
    	}
    }
    Yes you are right. I totally forget a C in the m_Packets.pop( )
    seems we got some major memory leaks

    --EDIT

    and this is how it should be

    Code:
    Clients :: ~Clients( )
    {
    	delete m_Socket;
    
    	m_Socket = NULL;
    
    	while(!m_CPackets.empty() )
    	{
    		delete m_CPackets.front();
    		m_CPackets.pop();
    	}
    	
    	while(!m_Packets.empty() )
    	{
    		m_Packets.pop();
    	}	
    }
    constructor :

    Code:
    class Clients
    {
    public:
    	CTCPSocket *m_Socket;
    	CChat *m_Chat;
    	CPDC *m_PDC;
    	string m_Name;
    	uint32_t m_Id;
    	uint32_t m_LastSend;
    	uint16_t m_Version;
    	string m_ChatRoom;
    	queue<BYTEARRAY> m_Packets;
    	queue<CCPacket*> m_CPackets;
             
             
            .... // more vars
            Clients( CTCPSocket *socket, CPDC *pdc );
            virtual ~Clients( );
    };
    Last edited by JacobNax; September 26th, 2011 at 07:15 AM.

  5. #20
    Join Date
    Apr 1999
    Posts
    27,449

    Re: Segmentation Fault ??

    Quote Originally Posted by JacobNax View Post
    heres some clue about the life-cycle of the Clients* object
    1) The code you posted doesn't show where you are setting pointers to NULL so that checking for NULL will work correctly. Deleting an object does not set an object to NULL.

    2) If your code is holding onto iterators or pointing to entries in the vector whenever the vector is resized (in this case grown), then those iterators or pointers can become invalidated here:
    Code:
    m_Clients.push_back(client);
    3) This loop could be rewritten using algorithms.
    Code:
    for(vector<Clients*> :: iterator i = m_Clients.begin( ); i != m_Clients.end( );)
    {
        if( (*i)->m_Socket && (*i)->m_Socket->GetConnected( ) && !(*i)->m_Socket->HasError( ) )
       {
             (*i)->m_Socket->SetFD(&fd, &send_fd, &nfds);
             nfds++;
             i++;
       }
       else
       {
            cout << "A client connection was lost." << endl;
           CleanUpUser((*i));
           i = m_Clients.erase( i );
       }
    }
    If the goal is to erase all closed connections in the loop, and to call SetFD on the good connections, that probably could have been accomplished this way:
    Code:
    #include <algorithm>
    //...
    bool IsGoodConnection(Client* pClient)
    {
       return  pClient->m_Socket && 
                   pClient->m_Socket->GetConnected( ) && 
                  !pClient->>m_Socket->HasError( );
    }
    
    struct FDSetter
    {
        int fd;
        int send_fd;
        int nfds;
    
        FDSetter() : nfds(1) {}
        void operator()(Client* pClient)
        {
            pClient->m_Socket->SetFD(&fd, &send_fd, &nfds);
            ++nfds;
        }
    };
    
    void SetFD(Client *pClient)
    {
         // good connections on left of partition, bad/closed connections on right of partition
        vector<Clients*>::iterator partition = std::stable_partition(m_Clients.begin(), m_Clients.end(), IsGoodConnection);
    
        // get the number of good entries (left of partition)
        nfds = std::distance(m_Clients.begin(), partition); 
    
       // for each good connection, call SetFD
       std::for_each(m_Clients.begin(), partition, FDSetter());
    
       // erase the bad connections by calling CleanUpUser and then erase them from the vector
       std::for_each(partition, m_Clients.end(), CleanUpUser);
       m_Clients.erase(partition, m_Clients.end();
    If what you wanted to do was to get rid of the closed connections, erase them from the vector, and keep/setup the good connections, then partitioning them first so that they are grouped accordingly could have been done. Note the use of stable_partition so as to maintain the order of the connections in the vector.

    Note that there are no loops. I did not know what to set fd and send_fd in the FDSetter class, since you didn't specify what they were or how they are used in the SetFD call. But the point is that writing loops as you've done can be error prone.

    Usage of algorithms guarantees that the code will work, provided that you call the correct algorithm functions with the correct arguments. In other words, there is no need for me to check the code above for iterator bugs or some other issue that could easily happen when attempting to write your own loops. It is also self-documenting, as any competent C++ programmer knows what those algorithm functions do. When given hand-coded loops as you've been doing, it may take several looks throught it to a) knows whart it does, and b) be assured it works correctly.

    The loop writing, erasing, making sure that iterators are seated properly, etc. -- leave that work to the algorithms.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; September 26th, 2011 at 10:15 AM.

  6. #21
    Join Date
    Sep 2011
    Posts
    13

    Re: Segmentation Fault ??

    thanks Paul
    your version looks neat and well written
    i haven't studied much the algorithms, just how the sort() and shuffling works.

Page 2 of 2 FirstFirst 12

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  





Click Here to Expand Forum to Full Width

Featured