From 4976bf8379a28e753cf0dc4883eb14287400d666 Mon Sep 17 00:00:00 2001 From: mwilliams Date: Wed, 27 Mar 2013 20:00:07 -0700 Subject: [PATCH] __lvalProxy needs to be request local But it was recently made thread local. If the value left in it at the end of one request was smart allocated, the next request would try to free it - but after its already been swept. We could make this RequestLocal, but that seems to add even more overhead. We could probably make it a __thread TypedValue, and just set it to uninit at the start of each request (and then tvAsVariant() where its used). This just puts it back to how it was, since global_variables() is already being dealt with appropriately. --- hphp/runtime/base/type_variant.cpp | 6 +- hphp/runtime/vm/name_value_table_wrapper.h | 1 + hphp/test/config-server.hdf | 2 +- hphp/test/test_server.cpp | 88 ++++++++++++++-------- hphp/test/test_server.h | 24 +++++- 5 files changed, 82 insertions(+), 39 deletions(-) diff --git a/hphp/runtime/base/type_variant.cpp b/hphp/runtime/base/type_variant.cpp index 04ba1a2a1..bc8962b74 100644 --- a/hphp/runtime/base/type_variant.cpp +++ b/hphp/runtime/base/type_variant.cpp @@ -2257,8 +2257,6 @@ public: static const bool CheckParams = true; }; -DECLARE_THREAD_LOCAL(Variant, __lvalProxy); - template Variant& Variant::LvalAtImpl0( Variant *self, T key, Variant *tmp, bool blackHole, ACCESSPARAMS_IMPL) { @@ -2304,7 +2302,7 @@ head: *tmp = self->getArrayAccess()->offsetGet(key); return *tmp; } - Variant& retv = *(__lvalProxy.get()); + Variant& retv = get_global_variables()->__lvalProxy; retv = self->getArrayAccess()->offsetGet(key); return retv; } @@ -2430,7 +2428,7 @@ Variant &Variant::lvalInvalid() { } Variant &Variant::lvalBlackHole() { - Variant &bh = *(__lvalProxy.get()); + Variant &bh = get_global_variables()->__lvalProxy; bh.unset(); return bh; } diff --git a/hphp/runtime/vm/name_value_table_wrapper.h b/hphp/runtime/vm/name_value_table_wrapper.h index c856d7a3a..acd008e76 100644 --- a/hphp/runtime/vm/name_value_table_wrapper.h +++ b/hphp/runtime/vm/name_value_table_wrapper.h @@ -165,6 +165,7 @@ class GlobalNameValueTableWrapper : public NameValueTableWrapper { Variant gvm_http_response_header; Variant __realPropProxy; + Variant __lvalProxy; Variant stgv_Variant[1]; #define k_SID stgv_Variant[0] diff --git a/hphp/test/config-server.hdf b/hphp/test/config-server.hdf index c1954e375..b5b18e4fa 100644 --- a/hphp/test/config-server.hdf +++ b/hphp/test/config-server.hdf @@ -6,7 +6,7 @@ Log { Server { Port = 8080 SourceRoot != echo $(pwd)/runtime/tmp - + ThreadCount = 1 AllowedFiles { 0 = string } diff --git a/hphp/test/test_server.cpp b/hphp/test/test_server.cpp index e1578b6e1..b2a1debcd 100644 --- a/hphp/test/test_server.cpp +++ b/hphp/test/test_server.cpp @@ -45,8 +45,9 @@ static int s_admin_port = 0; static int s_rpc_port = 0; static int inherit_fd = -1; -bool TestServer::VerifyServerResponse(const char *input, const char *output, - const char *url, const char *method, +bool TestServer::VerifyServerResponse(const char *input, const char **outputs, + const char **urls, int nUrls, + const char *method, const char *header, const char *postdata, bool responseHeader, const char *file /* = "" */, @@ -70,44 +71,54 @@ bool TestServer::VerifyServerResponse(const char *input, const char *output, AsyncFunc func(this, &TestServer::RunServer); func.start(); - String server = "http://"; - server += f_php_uname("n"); - server += ":" + lexical_cast(port) + "/"; - server += url; - string actual, err; - for (int i = 0; i < 10; i++) { - Variant c = f_curl_init(); - f_curl_setopt(c, k_CURLOPT_URL, server); - f_curl_setopt(c, k_CURLOPT_RETURNTRANSFER, true); - if (postdata) { - f_curl_setopt(c, k_CURLOPT_POSTFIELDS, postdata); - f_curl_setopt(c, k_CURLOPT_POST, true); - } - if (header) { - f_curl_setopt(c, k_CURLOPT_HTTPHEADER, CREATE_VECTOR1(header)); - } - if (responseHeader) { - f_curl_setopt(c, k_CURLOPT_HEADER, 1); - } + bool passed = true; - Variant res = f_curl_exec(c); - if (!same(res, false)) { - actual = res.toString(); - break; + string actual; + + int url = 0; + for (url = 0; url < nUrls; url++) { + String server = "http://"; + server += f_php_uname("n"); + server += ":" + lexical_cast(port) + "/"; + server += urls[url]; + actual = ""; + string err; + for (int i = 0; i < 10; i++) { + Variant c = f_curl_init(); + f_curl_setopt(c, k_CURLOPT_URL, server); + f_curl_setopt(c, k_CURLOPT_RETURNTRANSFER, true); + if (postdata) { + f_curl_setopt(c, k_CURLOPT_POSTFIELDS, postdata); + f_curl_setopt(c, k_CURLOPT_POST, true); + } + if (header) { + f_curl_setopt(c, k_CURLOPT_HTTPHEADER, CREATE_VECTOR1(header)); + } + if (responseHeader) { + f_curl_setopt(c, k_CURLOPT_HEADER, 1); + } + + Variant res = f_curl_exec(c); + if (!same(res, false)) { + actual = res.toString(); + break; + } + sleep(1); // wait until HTTP server is up and running + } + if (actual != outputs[url]) { + if (!responseHeader || + actual.find(outputs[url]) == string::npos) { + passed = false; + break; + } } - sleep(1); // wait until HTTP server is up and running } AsyncFunc(this, &TestServer::StopServer).run(); func.waitForEnd(); - bool passed = (actual == output); - if (responseHeader) { - passed = (actual.find(output) != string::npos); - } - if (!passed) { - printf("%s:%d\nParsing: [%s]\nBet %d:\n" + printf("%s:%d\nParsing: [%s] (req %d)\nBet %d:\n" "--------------------------------------\n" "%s" "--------------------------------------\n" @@ -115,7 +126,7 @@ bool TestServer::VerifyServerResponse(const char *input, const char *output, "--------------------------------------\n" "%s" "--------------------------------------\n", - file, line, input, (int)strlen(output), output, + file, line, input, url + 1, (int)strlen(outputs[url]), outputs[url], (int)actual.length(), actual.c_str()); return false; } @@ -204,6 +215,7 @@ bool TestServer::RunTests(const std::string &which) { RUN_TEST(TestInheritFdServer); RUN_TEST(TestSanity); RUN_TEST(TestServerVariables); + RUN_TEST(TestInteraction); RUN_TEST(TestGet); RUN_TEST(TestPost); RUN_TEST(TestCookie); @@ -274,6 +286,16 @@ bool TestServer::TestServerVariables() { return true; } +bool TestServer::TestInteraction() { + // run this twice to test lvalBlackHole + VSR2("