Skip to content

Commit

Permalink
[TIDESK-574] Fix HTTPServer crashing issue.
Browse files Browse the repository at this point in the history
Before we used the HTTPServerRequestHandler for the HTTPRequest
object we exposed into Kroll. The issue is this object's lifetime is
restricted by Poco to the handleRequest callback. Poco deletes this right after
this method returns. Once the GC of the interpreter runs it will try cleaning up
this HTTPRequest native object, but it's already deleted thus a crash results.

Moved HTTPRequest into it's own class to avoid the limited lifetime of the handler object.
Note: we may see issues with the request/response object if they are accessed outside the
callback's scope. Poco will most likely delete this after the handleRequest callback as well.
  • Loading branch information
fairwinds committed Oct 27, 2012
1 parent c12f424 commit 6b97e28
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 26 deletions.
19 changes: 1 addition & 18 deletions modules/ti.Network/protocols/http/http_server_request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@

namespace ti
{
HttpServerRequest::HttpServerRequest(Host *host, KMethodRef callback,
Poco::Net::HTTPServerRequest& request) :
HttpServerRequest::HttpServerRequest(Poco::Net::HTTPServerRequest& request) :
StaticBoundObject("Network.HttpServerRequest"),
host(host),
callback(callback),
request(request)
{
/**
Expand Down Expand Up @@ -80,20 +77,6 @@ namespace ti
{
}

void HttpServerRequest::handleRequest(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse &response)
{
ValueList args;

// We MUST duplicate 'this' before casting it into an
// AutoPtr or else we will free this memory at the wrong time.
this->duplicate();
AutoPtr<HttpServerRequest> autoThis = this;

args.push_back(Value::NewObject(autoThis));
args.push_back(Value::NewObject(new HttpServerResponse(response)));
RunOnMainThread(callback, args);
}

void HttpServerRequest::GetMethod(const ValueList& args, KValueRef result)
{
std::string method = request.getMethod();
Expand Down
8 changes: 2 additions & 6 deletions modules/ti.Network/protocols/http/http_server_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@

namespace ti
{
class HttpServerRequest : public Poco::Net::HTTPRequestHandler, public StaticBoundObject
class HttpServerRequest : public StaticBoundObject
{
public:
HttpServerRequest(Host *host, KMethodRef callback, Poco::Net::HTTPServerRequest& request);
HttpServerRequest(Poco::Net::HTTPServerRequest& request);
virtual ~HttpServerRequest();

void handleRequest(Poco::Net::HTTPServerRequest &request, Poco::Net::HTTPServerResponse& response);

private:
Host *host;
KMethodRef callback;
Poco::Net::HTTPServerRequest& request;

void GetMethod(const ValueList& args, KValueRef result);
Expand Down
28 changes: 26 additions & 2 deletions modules/ti.Network/protocols/http/http_server_request_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,34 @@
*/

#include "http_server_request.h"
#include "http_server_response.h"
#include "http_server_request_factory.h"

namespace ti
{
class HTTPRequestHandler : public Poco::Net::HTTPRequestHandler {
public:
HTTPRequestHandler(KMethodRef callback)
: m_callback(callback)
{
}

virtual void handleRequest(Poco::Net::HTTPServerRequest&, Poco::Net::HTTPServerResponse&);

private:
KMethodRef m_callback;
};

void HTTPRequestHandler::handleRequest(Poco::Net::HTTPServerRequest& request, Poco::Net::HTTPServerResponse& response) {
// XXX(Josh): The request and response object's lifetime is limited to this functions call.
// If the developer should keep a reference to these around past the callback lifetime and then
// attempts to access it may result in a crash!
ValueList args;
args.push_back(Value::NewObject(new HttpServerRequest(request)));
args.push_back(Value::NewObject(new HttpServerResponse(response)));
RunOnMainThread(m_callback, args);
}

HttpServerRequestFactory::HttpServerRequestFactory(Host *host, KMethodRef callback) :
host(host),
callback(callback)
Expand All @@ -20,8 +44,8 @@ namespace ti
}

Poco::Net::HTTPRequestHandler* HttpServerRequestFactory::createRequestHandler(
const Poco::Net::HTTPServerRequest& request)
const Poco::Net::HTTPServerRequest& request)
{
return new HttpServerRequest(host, callback, const_cast<Poco::Net::HTTPServerRequest&>(request));
return new HTTPRequestHandler(callback);
}
}

0 comments on commit 6b97e28

Please sign in to comment.