I'm trying to learn C++ using "MUD Game Programming" and I am working through the examples, but when I try to erase a connection from a vector I get an error: "Aborted (core dumped)." This usually happens when erasing the last one from the vector. I have tried solutions such as checking to see if the iterator != connlist.end() before deleting, and trying to use connlist.clear() if there is only one element left in the list, but these don't seem to help.
I am using g++ to compile and working on Cygwin.
The error happens at connlist.erase(), which you can see is called when the user types "quit."
void NMudServer::StartListening() {
fd_set read_set;
std::vector<SocketLib::Connection>::iterator itr;
std::vector<SocketLib::Connection>::iterator itr2;
TIMEVAL zerotime;
zerotime.tv_usec = 0;
zerotime.tv_sec = 0;
char buffer[buf_len];
int err;
lsock.Listen( default_port );
if( lsock.IsListening() ) {
std::cout << "Telnet listening on port " << default_port << "." << std::endl;
} else {
std::cout << "Could not start Telnet listening socket! - Last error code: " << WSAGetLastError() << std::endl;
return;
}
while( lsock.IsListening() ) {
FD_ZERO( &read_set );
FD_SET( lsock.GetSock(), &read_set );
for( itr = connlist.begin(); itr != connlist.end(); ++itr ) {
FD_SET( itr->GetSock(), &read_set );
}
int sel = select( 0x7FFFFFFF, &read_set, NULL, NULL, &zerotime );
if( sel > 0 ) {
if( FD_ISSET( lsock.GetSock(), &read_set ) ) {
SocketLib::DataSocket dsock = lsock.Accept();
SocketLib::Connection conn( dsock, buf_len );
connlist.push_back( conn );
conn.Send( "Hello!\r\n", 8 );
}
for( itr = connlist.begin(); itr != connlist.end(); ++itr ) {
if( FD_ISSET( itr->GetSock(), &read_set ) ) {
err = itr->Receive();
if( err == -1 ) {
std::cout << "Socket receiving error!" << std::endl;
std::cout << "Error code: " << WSAGetLastError() << std::endl;
std::cout << "Exiting due to error." << std::endl;
CloseAllConnections(); // This is when the connection is closed. Need to only close the one connection.
break;
} else if( err == 0 ) {
itr->Close();
connlist.erase( itr );
--itr;
} else if( itr->IsReady() ) {
int size;
size = itr->GetData( buffer );
if( strcmp( buffer, "servquit\r\n" ) == 0 ) {
CloseAllConnections();
} else if( strcmp( buffer, "quit\r\n" ) == 0 ) {
itr->Close();
connlist.erase( itr ); // When you go to erase the last element, it errors
--itr;
} else {
// Echo back the data to all connections
for( itr2 = connlist.begin(); itr2 != connlist.end(); ++itr2 ) {
if( itr2->GetSock() != itr->GetSock() ) {
int err2;
itr2->Send( buffer, size );
if( err2 == -1 ) {
std::cout << "Socket sending error: " << WSAGetLastError() << std::endl;
}
} else {
itr2->Send( "\r\n", 2 );
}
} // end for
}// end if-else
itr->Reset();
} // end if-else-else
} // end if
} // end for
} // end if sel
} // end while
}
void NMudServer::CloseAllConnections() {
lsock.Close();
std::vector<SocketLib::Connection>::iterator itr;
for( itr = connlist.begin(); itr != connlist.end(); ++itr ) {
itr->Close();
}
WSACleanup();
}
For reference, the book is: Penton, Ron. MUD Game Programming. Boston, MA, USA: Course Technology / Cengage Learning, 2003.
You are iterating a vector and erasing elements from it inside the loop. That's never a good idea. vector::erase
invalidates the iterator you then use to iterate further.
See: https://en.cppreference.com/w/cpp/container/vector/erase
Reason for this is that a vector might reallocate and/or move elements when you erase an element.
Just close those connections inside that loop and perform an erase-remove afterwards:
connlist.erase(std::remove_if(connlist.begin(), connlist.end(), is_closed), connlist.end());
with is_closed
being something like:
is_closed = [](const SocketLib::Connection& c) {return !c.open();};
PS: I don't know that SocketLib
so you might change that is_closed
a bit
PPS: use range-based for loops like for(auto& connection : connlist) {...}
As mentioned in a comment, the vector will not reallocate in this case, the other points still stand.
User contributions licensed under CC BY-SA 3.0