Add Locking calls to file_io.h plus implementations and test

util/test/thread.h

+// Copyright 2015 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef CRASHPAD_UTIL_TEST_THREAD_H_
+#define CRASHPAD_UTIL_TEST_THREAD_H_
+
+#include "build/build_config.h"

[Mark Mentovai, 2015/03/20 15:03:56]

build > base

[scottmg, 2015/03/20 21:07:35]

Done.

+#include "base/basictypes.h"
+
+#if defined(OS_POSIX)
+#include <pthread.h>
+#elif defined(OS_WIN)
+#include <windows.h>
+#endif  // OS_POSIX
+
+namespace crashpad {
+namespace test {
+
+template <class T>
+class Thread {

[Mark Mentovai, 2015/03/20 15:42:12]

Provide at least a //! \brief for the class and all of the public methods. For
Join(), it’d be good to call out which threads it can validly be called on: any
thread other than the one that was started vs. just the thread that called
Start().

[scottmg, 2015/03/20 21:07:35]

Done.

+ public:
+  Thread() : entry_(nullptr), data_(nullptr), thread_(nullptr) {}

[Mark Mentovai, 2015/03/20 15:03:56]

nullptr isn’t a valid pthread_t on all systems. 0 or a static const
kInvalidThreadHandle = 0 would work.

Nothing requires 0 to be an invalid pthread_t, although it’s true on all of the
systems we will care about. POSIX is introducing PTHREAD_NULL, but it’s not in
the spec yet and none of our systems have it yet either.

[Mark Mentovai, 2015/03/20 15:42:12]

~Thread() should either EXPECT that if Start() was called, Join() was also
called. OR it should Join() if there was a Start() without a Join(). I prefer
the former.

This will catch bad uses of Thread where they happen, so that an errant thread
left alive in one test can’t ruin the state for future tests.

[scottmg, 2015/03/20 21:07:35]

Done.

[scottmg, 2015/03/20 21:07:35]

Done.

+  void Start(void (*entry)(T*), T* data);

[Mark Mentovai, 2015/03/20 15:03:56]

The function pointer is a very C-like interface, with a thin C++ template above
it to try to mask it.

How about if instead, you made Thread a base class that calls an unimplemented
virtual Run() method? Then all of the data is encapsulated in subclass objects,
so you don’t need to deal with the data argument pointer.

That also lets the implementation move to a .cc file and avoids having multiple
copies of the same thing each time the template gets instantiated with a
different T. You could also separate _win.cc from _posix.cc.

[Mark Mentovai, 2015/03/20 15:42:12]

I wrote:
Run() would be a pretty bad name, but you get the idea.

[scottmg, 2015/03/20 21:07:35]

Main()?

[scottmg, 2015/03/20 21:07:35]

Done.

+  void Join();

[Mark Mentovai, 2015/03/20 15:42:12]

Similarly, Start() should EXPECT that it hasn’t already been called.

[scottmg, 2015/03/20 21:07:35]

Done.

+
+  T* data() { return data_; }
+
+ private:
+  static
+#if defined(OS_POSIX)
+      void*
+#elif defined(OS_WIN)
+      DWORD WINAPI
+#endif  // OS_POSIX
+      ThreadEntryThunk(void* argument);
+
+  void (*entry_)(T*);
+  T* data_;
+
+#if defined(OS_POSIX)
+  pthread_t thread_;
+#elif defined(OS_WIN)
+  HANDLE thread_;
+#endif
+
+  DISALLOW_COPY_AND_ASSIGN(Thread);
+};
+
+template <class T>
+void Thread<T>::Start(void (*entry)(T*), T* data) {
+  entry_ = entry;
+  data_ = data;
+#if defined(OS_POSIX)
+  int rv = pthread_create(&thread_, nullptr, ThreadEntryThunk, this);
+  ASSERT_EQ(0, rv) << ErrnoMessage(rv, "pthread_create");
+#elif defined(OS_WIN)
+  thread_ = CreateThread(nullptr, 0, ThreadEntryThunk, this, 0, nullptr);
+  ASSERT_NE(nullptr, thread_) << ErrorMessage("CreateThread");
+#endif  // OS_POSIX
+}
+
+template <class T>
+void Thread<T>::Join() {
+#if defined(OS_POSIX)
+  int rv = pthread_join(thread_, nullptr);
+  ASSERT_EQ(0, rv) << ErrnoMessage(rv, "pthread_join");
+#elif defined(OS_WIN)
+  DWORD result = WaitForSingleObject(thread_, INFINITE);
+  EXPECT_EQ(WAIT_OBJECT_0, result) << ErrorMessage("WaitForSingleObject");
+#endif  // OS_POSIX
+}
+
+// static
+template <class T>
+#if defined(OS_POSIX)
+void*
+#elif defined(OS_WIN)
+DWORD WINAPI
+#endif  // OS_POSIX
+Thread<T>::ThreadEntryThunk(void* argument) {
+  Thread* self = reinterpret_cast<Thread*>(argument);
+  self->entry_(self->data_);
+
+#if defined(OS_POSIX)
+  return nullptr;
+#elif defined(OS_WIN)
+  return 0;
+#endif  // OS_POSIX
+}
+
+}  // namespace test
+}  // namespace crashpad
+
+#endif  // CRASHPAD_UTIL_TEST_THREAD_H_