Debug builds may abort() when revalidating expired output resources

As a workaround, remove the LOG(DFATAL) and replace it with a TODO. #1553
apache · Jun 5, 2017 · e1eabb3 · e1eabb3
1 parent c122121
commit e1eabb3
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 2 deletions.
diff --git a/net/instaweb/rewriter/output_resource.cc b/net/instaweb/rewriter/output_resource.cc
@@ -221,8 +221,10 @@ void OutputResource::SetHash(StringPiece hash) {
 void OutputResource::LoadAndCallback(NotCacheablePolicy not_cacheable_policy,
                                      const RequestContextPtr& request_context,
                                      AsyncCallback* callback) {
-  LOG(DFATAL) << "Output resources shouldn't be loaded via "
-                 "LoadAsync, but rather through FetchResource";
+  // TODO(oschaaf): Output resources shouldn't be loaded via LoadAsync, but
+  // rather through FetchResource. ProxyInterfaceTest.TestAbortOnRevalidation
+  // does manage to hit this code, which caused an abort in debug builds.
+  // See https://github.com/pagespeed/mod_pagespeed/issues/1553
   callback->Done(false /* lock_failure */, writing_complete_);
 }
 

diff --git a/pagespeed/automatic/proxy_interface_test.cc b/pagespeed/automatic/proxy_interface_test.cc
@@ -381,6 +381,46 @@ TEST_F(ProxyInterfaceTest, LoggingInfo) {
   EXPECT_TRUE(logging_info()->is_request_disabled());
 }
 
+// Regression test for https://github.com/pagespeed/mod_pagespeed/issues/1553
+TEST_F(ProxyInterfaceTest, TestNoDebugAbortAfterMoreThenOneYear) {
+  GoogleString text;
+  RequestHeaders request_headers;
+  ResponseHeaders response_headers;
+  response_headers.Add(HttpAttributes::kContentType,
+		       kContentTypeHtml.mime_type());
+  response_headers.SetStatusAndReason(HttpStatus::kOK);
+
+  GoogleString url = "http://test.com/flush_subresources.html"
+    "?PageSpeedFilters=+extend_cache_css,-inline_css";
+  mock_url_fetcher_.SetResponse("http://test.com/flush_subresources.html",
+				response_headers,
+                                "<html><head><link rel='stylesheet' "
+				"type='text/css' href='test.css'></head></html>");
+
+  response_headers.Replace(HttpAttributes::kContentType,
+			   kContentTypeCss.mime_type());
+  mock_url_fetcher_.SetResponse("http://test.com/test.css", response_headers,
+                                kCssContent);
+
+  FetchFromProxy(url,
+                 request_headers,
+                 true,  /* expect_success */
+                 &text,
+                 &response_headers,
+                 false  /* proxy_fetch_property_callback_collector_created */);
+
+  SetTimeMs(MockTimer::kApr_5_2010_ms + 2 * Timer::kYearMs);
+
+  // This second fetch would end up with VLOG(DFATAL)
+  FetchFromProxy(url,
+                 request_headers,
+                 true,  /* expect_success */
+                 &text,
+                 &response_headers,
+		 false  /* proxy_fetch_property_callback_collector_created */);
+}
+
+
 TEST_F(ProxyInterfaceTest, SkipPropertyCacheLookupIfOptionsNotEnabled) {
   GoogleString url = "http://www.example.com/";
   GoogleString text;