From grdetil@scrc.umanitoba.ca Tue Jul 3 15:13:55 2001 Date: Tue, 3 Jul 2001 17:00:36 -0500 (CDT) From: Gilles Detillieux To: "ht://Dig mailing list" Subject: [htdig] PATCH: fix segfaults on htdig-3.2.0b4-070101 snapshot Hi, folks. Below is a patch for the latest 3.2 snapshot, which fixes a bug that's been around probably since March, when the HtHTTP class was subdivided. There was some pretty sloppy pointer management, that caused the allocation heap to get messed up, and eventually caused a segmentation fault. In the process of tracking this down, I found and fixed a few other things. I've committed all this to CVS, so it'll be in next Sunday's snapshot. I'm still extremely suspicious of the Transport_Response::Reset() method, and its deleting of _modification_time and _access_time fields. I see potential there for all sorts of problems too, as the _modification_time fields isn't necessarily set by a new HtDateTime object all the time. Also Transport::SetRequestModificationTime() sets the pointer without deleting the previous value, so there's a big inconsistency in how this field is handled, and probably a memory leak here as well. Can someone explain to me why on earth these fields are handled as pointers in the first place, and which calling convention should be used for passing HtDateTime objects to these classes? I haven't been able to do anything about this yet, but at least this patch fixes the more immediate and serious problem with the HtHTTP class. Apply this patch in the main htdig-3.2.0b4-070101 source directory using "patch -p1 < this-message". diff -rup htdig-3.2.0b4-070101/htdig/Retriever.cc htdig-3.2.0b4-070101.patched/htdig/Retriever.cc --- htdig-3.2.0b4-070101/htdig/Retriever.cc Wed May 16 12:43:09 2001 +++ htdig-3.2.0b4-070101.patched/htdig/Retriever.cc Tue Jul 3 12:09:38 2001 @@ -1009,6 +1009,7 @@ Retriever::GetLocal(const String &strurl static StringList *paths = 0; StringList *defaultdocs = 0; URL aUrl(url); + url = aUrl.get(); // make sure we look at a parsed URL // // Initialize prefix/path list if this is the first time. diff -rup htdig-3.2.0b4-070101/htnet/Connection.cc htdig-3.2.0b4-070101.patched/htnet/Connection.cc --- htdig-3.2.0b4-070101/htnet/Connection.cc Fri May 4 07:42:33 2001 +++ htdig-3.2.0b4-070101.patched/htnet/Connection.cc Tue Jul 3 13:41:56 2001 @@ -62,8 +62,8 @@ Connection::Connection() { sock = -1; connected = 0; - peer = 0; - server_name = 0; + peer = ""; + server_name = ""; all_connections.Add(this); timeout_value = 0; retry_value = 1; @@ -89,8 +89,8 @@ Connection::Connection(int socket) { perror("getpeername"); } - peer = 0; - server_name = 0; + peer = ""; + server_name = ""; all_connections.Add(this); timeout_value = 0; retry_value = 1; @@ -106,8 +106,6 @@ Connection::~Connection() { all_connections.Remove(this); this->Close(); - delete peer; - free(server_name); } @@ -251,8 +249,7 @@ int Connection::Assign_Server(const Stri memcpy((char *)&server.sin_addr, (char *)&addr, sizeof(addr)); } - if(server_name) free(server_name); - server_name = strdup(name); + server_name = name.get(); return OK; } @@ -713,7 +710,7 @@ extern "C" char *inet_ntoa(struct in_add // char *Connection::Get_Peername() { - if (!peer) + if (peer.empty()) { struct sockaddr_in p; GETPEERNAME_LENGTH_T length = sizeof(p); @@ -727,11 +724,11 @@ char *Connection::Get_Peername() length = sizeof(p.sin_addr); hp = gethostbyaddr((const char *) &p.sin_addr, length, AF_INET); if (hp) - peer = strdup((char *) hp->h_name); + peer = (char *) hp->h_name; else - peer = strdup((char *) inet_ntoa(p.sin_addr)); + peer = (char *) inet_ntoa(p.sin_addr); } - return peer; + return peer.get(); } @@ -740,14 +737,14 @@ char *Connection::Get_Peername() // char *Connection::Get_PeerIP() { - struct sockaddr_in peer; - GETPEERNAME_LENGTH_T length = sizeof(peer); + struct sockaddr_in p; + GETPEERNAME_LENGTH_T length = sizeof(p); - if (getpeername(sock, (struct sockaddr *) &peer, &length) < 0) + if (getpeername(sock, (struct sockaddr *) &p, &length) < 0) { return 0; } - return inet_ntoa(peer.sin_addr); + return inet_ntoa(p.sin_addr); } #ifdef NEED_PROTO_GETHOSTNAME diff -rup htdig-3.2.0b4-070101/htnet/Connection.h htdig-3.2.0b4-070101.patched/htnet/Connection.h --- htdig-3.2.0b4-070101/htnet/Connection.h Sat Mar 17 16:45:49 2001 +++ htdig-3.2.0b4-070101.patched/htnet/Connection.h Fri Jun 29 17:58:39 2001 @@ -19,6 +19,7 @@ #define _Connection_h_ #include "Object.h" +#include "htString.h" #include #include @@ -26,7 +27,7 @@ #include #include -class String; +//class String; class Connection : public Object { @@ -54,7 +55,7 @@ public: // Host stuff int Assign_Server(const String& name); int Assign_Server(unsigned int addr = INADDR_ANY); - char *Get_Server() {return server_name;} + char *Get_Server() {return server_name.get();} // Connection establishment virtual int Connect(); @@ -104,8 +105,8 @@ private: int sock; struct sockaddr_in server; int connected; - char *peer; - char *server_name; + String peer; + String server_name; int need_io_stop; int timeout_value; int retry_value; diff -rup htdig-3.2.0b4-070101/htnet/HtHTTP.cc htdig-3.2.0b4-070101.patched/htnet/HtHTTP.cc --- htdig-3.2.0b4-070101/htnet/HtHTTP.cc Fri May 4 07:42:33 2001 +++ htdig-3.2.0b4-070101.patched/htnet/HtHTTP.cc Tue Jul 3 13:42:27 2001 @@ -124,7 +124,8 @@ HtHTTP::HtHTTP() _useproxy = 0; // Create a new Connection object, as long as HTTP needs a TCP conn - _connection = new Connection(); + // This is now done in HtHTTPBasic or HtHTTPSecure constructor... + //_connection = new Connection(); } @@ -133,8 +134,10 @@ HtHTTP::HtHTTP() HtHTTP::~HtHTTP() { // Let's delete the connection object + CloseConnection(); if (_connection) delete (_connection); + _connection = 0; } diff -rup htdig-3.2.0b4-070101/htnet/HtHTTPBasic.cc htdig-3.2.0b4-070101.patched/htnet/HtHTTPBasic.cc --- htdig-3.2.0b4-070101/htnet/HtHTTPBasic.cc Sat Mar 17 16:45:49 2001 +++ htdig-3.2.0b4-070101.patched/htnet/HtHTTPBasic.cc Tue Jul 3 13:42:51 2001 @@ -43,6 +43,9 @@ HtHTTPBasic::~HtHTTPBasic() { // Free up the Connection // - delete _connection; + CloseConnection(); + if (_connection) + delete _connection; + _connection = 0; } diff -rup htdig-3.2.0b4-070101/htnet/HtHTTPSecure.cc htdig-3.2.0b4-070101.patched/htnet/HtHTTPSecure.cc --- htdig-3.2.0b4-070101/htnet/HtHTTPSecure.cc Sat Mar 17 16:45:49 2001 +++ htdig-3.2.0b4-070101.patched/htnet/HtHTTPSecure.cc Tue Jul 3 13:42:59 2001 @@ -45,7 +45,10 @@ HtHTTPSecure::~HtHTTPSecure() { // Free up the connection // - delete _connection; + CloseConnection(); + if (_connection) + delete _connection; + _connection = 0; } #endif diff -rup htdig-3.2.0b4-070101/htnet/HtNNTP.cc htdig-3.2.0b4-070101.patched/htnet/HtNNTP.cc --- htdig-3.2.0b4-070101/htnet/HtNNTP.cc Sat Mar 17 16:45:49 2001 +++ htdig-3.2.0b4-070101.patched/htnet/HtNNTP.cc Tue Jul 3 13:44:43 2001 @@ -109,7 +109,10 @@ HtNNTP::~HtNNTP() { // Free the connection // - delete _connection; + CloseConnection(); + if (_connection) + delete _connection; + _connection = 0; } diff -rup htdig-3.2.0b4-070101/htnet/Transport.cc htdig-3.2.0b4-070101.patched/htnet/Transport.cc --- htdig-3.2.0b4-070101/htnet/Transport.cc Fri May 4 07:42:33 2001 +++ htdig-3.2.0b4-070101.patched/htnet/Transport.cc Tue Jul 3 13:41:40 2001 @@ -309,8 +309,11 @@ int Transport::CloseConnection() { if( _connection == 0 ) { - cout << "Transport::CloseConnection: _connection is NULL\n"; - exit(0); + // We can't treat this as a fatal error, because CloseConnection() + // may be called from our destructor after _connection already deleted. +// cout << "Transport::CloseConnection: _connection is NULL\n"; +// exit(0); + return 0; } if(_connection->IsOpen()) -- Gilles R. Detillieux E-mail: Spinal Cord Research Centre WWW: http://www.scrc.umanitoba.ca/~grdetil Dept. Physiology, U. of Manitoba Phone: (204)789-3766 Winnipeg, MB R3E 3J7 (Canada) Fax: (204)789-3930 _______________________________________________ htdig-general mailing list To unsubscribe, send a message to with a subject of unsubscribe FAQ: http://htdig.sourceforge.net/FAQ.html