From b4e827e01b5197b0c3158eb87db5c7e8596a9377 Mon Sep 17 00:00:00 2001
From: KongQun Yang <kqyang@google.com>
Date: Mon, 6 Aug 2018 16:11:57 -0700
Subject: [PATCH] Make sure TempFilePath always generate unique file path

Previously it is possible that the same file path is generated when
the function is called consecutively in the same thread.

The problem can be re-produced in Windows. Does not seem to be
re-producible in Linux and Mac.

Fixes #448, #449.

Change-Id: Ia0163492e3494eba00f56f4d356aa1010b9660cc
---
 packager/file/file_util.cc          | 16 ++++++++++++----
 packager/file/file_util_unittest.cc | 20 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/packager/file/file_util.cc b/packager/file/file_util.cc
index 7eb4e55546..9f8304d3e5 100644
--- a/packager/file/file_util.cc
+++ b/packager/file/file_util.cc
@@ -19,11 +19,19 @@ namespace shaka {
 namespace {
 // Create a temp file name using process id, thread id and current time.
 std::string TempFileName() {
-  const int32_t pid = static_cast<int32_t>(base::GetCurrentProcId());
-  const int32_t tid = static_cast<int32_t>(base::PlatformThread::CurrentId());
+  const int32_t process_id = static_cast<int32_t>(base::GetCurrentProcId());
+  const int32_t thread_id =
+      static_cast<int32_t>(base::PlatformThread::CurrentId());
+
+  // We may need two or more temporary files in the same thread. There might be
+  // name collision if they are requested around the same time, e.g. called
+  // consecutively. Use a thread_local instance to avoid that.
+  static thread_local int32_t instance_id = 0;
+  ++instance_id;
+
   const int64_t current_time = base::Time::Now().ToInternalValue();
-  return base::StringPrintf("packager-tempfile-%x-%x-%" PRIx64, pid, tid,
-                            current_time);
+  return base::StringPrintf("packager-tempfile-%x-%x-%x-%" PRIx64, process_id,
+                            thread_id, instance_id, current_time);
 }
 }  // namespace
 
diff --git a/packager/file/file_util_unittest.cc b/packager/file/file_util_unittest.cc
index 188ac21cc5..f6b76faf98 100644
--- a/packager/file/file_util_unittest.cc
+++ b/packager/file/file_util_unittest.cc
@@ -8,15 +8,33 @@
 
 #include <gtest/gtest.h>
 
+#include "packager/base/logging.h"
+
 namespace shaka {
 
-TEST(FileUtilTest, Basic) {
+TEST(FileUtilTest, TempFilePathInDesignatedDirectory) {
   std::string temp_file_path;
   EXPECT_TRUE(TempFilePath("test", &temp_file_path));
   EXPECT_EQ(temp_file_path.find("test"), 0u);
+  LOG(INFO) << "temp file path: " << temp_file_path;
+}
 
+TEST(FileUtilTest, TempFilePathInSystemTempDirectory) {
+  std::string temp_file_path;
   EXPECT_TRUE(TempFilePath("", &temp_file_path));
   // temp_file_path should be created in a system specific temp directory.
+  LOG(INFO) << "temp file path: " << temp_file_path;
+}
+
+TEST(FileUtilTest, TempFilePathCalledTwice) {
+  const char kTempDir[] = "/test/";
+  std::string temp_file_path1;
+  std::string temp_file_path2;
+  ASSERT_TRUE(TempFilePath(kTempDir, &temp_file_path1));
+  ASSERT_TRUE(TempFilePath(kTempDir, &temp_file_path2));
+  ASSERT_NE(temp_file_path1, temp_file_path2);
+  LOG(INFO) << "temp file path1: " << temp_file_path1;
+  LOG(INFO) << "temp file path2: " << temp_file_path2;
 }
 
 }  // namespace shaka