Hello there, I'm currently building my own, specialized http-server based on the http-server example. I experience crashes for certain types of replies. Here's something I really do not understand: In connection.cpp, after request_parser_.parse() finished and result == true, there is this line: boost::asio::async_write(socket_, reply_.to_buffers(), boost::bind(&connection::handle_write, shared_from_this(), boost::asio::placeholders::error)); It's the last user-code in the stacktrace of the crash that I frequently experience. What bothers me is the reply_.to_buffers() function. It looks like this: std::vectorboost::asio::const_buffer reply::to_buffers() { std::vectorboost::asio::const_buffer buffers; buffers.push_back(status_strings::to_buffer(status)); for (std::size_t i = 0; i < headers.size(); ++i) { header& h = headers[i]; buffers.push_back(boost::asio::buffer(h.name)); buffers.push_back(boost::asio::buffer(misc_strings::name_value_separator)); buffers.push_back(boost::asio::buffer(h.value)); buffers.push_back(boost::asio::buffer(misc_strings::crlf)); } buffers.push_back(boost::asio::buffer(misc_strings::crlf)); buffers.push_back(boost::asio::buffer(content)); return buffers; } How can this even work? It adds temporaries (the misc_strings are const char[]s) to a const_buffer-vector and reads them later. Even the references to h.name, etc. seem invalid to me, since the io is asynchronous and there's no guarantee that they don't change. IMHO the whole content needs to be COPIED to new buffers and then, after the async_write finished, deleted. Am I missing something? Was the author of this example not aware of the const_buffer's ownership policy (i.e. it is *not* owner of the data)? I've spent hours and hours in debugging now, but I just can't figure out where it goes wrong. You can see the stacktrace here: http://mandrill.fuxx0r.net/c++/paste:7702 Kind regards, Daniel Albuschat -- eat(this); // delicious suicide
Daniel Albuschat wrote:
I experience crashes for certain types of replies.
The "crash" you see is asio's buffer debug checking in action. For whatever reason, it thinks one of the strings referred to in a buffer is no longer valid. What is different about those "certain types of replies"?
What bothers me is the reply_.to_buffers() function. It looks like this:
std::vectorboost::asio::const_buffer reply::to_buffers() { std::vectorboost::asio::const_buffer buffers; buffers.push_back(status_strings::to_buffer(status)); for (std::size_t i = 0; i < headers.size(); ++i) { header& h = headers[i]; buffers.push_back(boost::asio::buffer(h.name)); buffers.push_back(boost::asio::buffer(misc_strings::name_value_separator)); buffers.push_back(boost::asio::buffer(h.value)); buffers.push_back(boost::asio::buffer(misc_strings::crlf)); } buffers.push_back(boost::asio::buffer(misc_strings::crlf)); buffers.push_back(boost::asio::buffer(content)); return buffers; }
How can this even work? It adds temporaries (the misc_strings are const char[]s) to a const_buffer-vector and reads them later. Even the references to h.name, etc. seem invalid to me, since the io is asynchronous and there's no guarantee that they don't change.
The buffers in the vector refer directly to the strings contained within the reply object (in addition to the misc_strings, of course). The reply object must not be modified or destroyed for as long as the buffers returned by reply::to_buffers() are being used by an asynchronous operation. In the HTTP server example, the reply object is a data member of the connection class, so that it remains valid until the connection is closed.
IMHO the whole content needs to be COPIED to new buffers and then, after the async_write finished, deleted.
Am I missing something? Was the author of this example not aware of the const_buffer's ownership policy (i.e. it is *not* owner of the data)?
I pretty sure I was aware ;) However that's not to say there are no bugs in the HTTP server or buffer debug checking. Cheers, Chris
2007/6/13, Christopher Kohlhoff
Daniel Albuschat wrote:
I experience crashes for certain types of replies.
The "crash" you see is asio's buffer debug checking in action. For whatever reason, it thinks one of the strings referred to in a buffer is no longer valid.
I'm pretty sure it actually *is* invalid.
What is different about those "certain types of replies"?
I could not trace that down. I've rewritten that part to be synchronous, so I do not have the lifetime-problem. There's a reply::to_string() function now: std::string reply::to_string() const { std::stringstream ss; ss << status_strings::to_string(status); for (std::size_t i = 0; i < headers.size(); ++i) { const header& h = headers[i]; ss << h.name << misc_strings::name_value_separator << h.value << misc_strings::crlf; } ss << misc_strings::crlf; ss << content; return ss.str(); } And I've changed the write-part of connection::handle_read as follows: request_handler_.handle_request(request_, reply_); std::string s = reply_.to_string(); boost::asio::write(socket_,boost::asio::buffer(s.data(),s.size())); connection_manager_.stop(shared_from_this()); It's not the optimal solution, but it's safe.
IMHO the whole content needs to be COPIED to new buffers and then, after the async_write finished, deleted.
Am I missing something? Was the author of this example not aware of the const_buffer's ownership policy (i.e. it is *not* owner of the data)?
I pretty sure I was aware ;) However that's not to say there are no bugs in the HTTP server or buffer debug checking.
As the reply is local to the connection, this should indeed work. I don't know where the problem lies exactly, probably it is a bug introduced by me at a different code-location. The server runs stable now, and I'll leave it at that. Since you seem to be the auther of the example: BIG THANKS to you, it really saved me MUCH time and I have a very very neat solution to connect my PHP script with the program-logic/database now. Regards, Daniel Albuschat -- eat(this); // delicious suicide
participants (2)
-
Christopher Kohlhoff -
Daniel Albuschat