Skip to content

Commit

Permalink
feat: use standard threads
Browse files Browse the repository at this point in the history
  • Loading branch information
sergiud committed Jan 3, 2024
1 parent 645d0a5 commit 066300c
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 232 deletions.
19 changes: 0 additions & 19 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ set (CMAKE_POSITION_INDEPENDENT_CODE ON)
set (CMAKE_VISIBILITY_INLINES_HIDDEN ON)

set (CMAKE_DEBUG_POSTFIX d)
set (CMAKE_THREAD_PREFER_PTHREAD 1)

find_package (GTest NO_MODULE)

Expand Down Expand Up @@ -159,24 +158,6 @@ check_cxx_symbol_exists (backtrace_symbols execinfo.h
HAVE_EXECINFO_BACKTRACE_SYMBOLS)
check_cxx_symbol_exists (_chsize_s io.h HAVE__CHSIZE_S)

cmake_push_check_state (RESET)

set (CMAKE_REQUIRED_DEFINITIONS -D_XOPEN_SOURCE=500)

if (Threads_FOUND)
set (CMAKE_REQUIRED_LIBRARIES Threads::Threads)
endif (Threads_FOUND)

check_cxx_symbol_exists (pthread_threadid_np pthread.h HAVE_PTHREAD_THREADID_NP)

cmake_pop_check_state ()

if (HAVE_RWLOCK_INIT AND HAVE_RWLOCK_RDLOCK AND HAVE_RWLOCK_WRLOCK AND
HAVE_RWLOCK_UNLOCK AND HAVE_RWLOCK_DESTROY)
set (HAVE_RWLOCK TRUE)
endif (HAVE_RWLOCK_INIT AND HAVE_RWLOCK_RDLOCK AND HAVE_RWLOCK_WRLOCK AND
HAVE_RWLOCK_UNLOCK AND HAVE_RWLOCK_DESTROY)

cmake_push_check_state (RESET)
set (CMAKE_REQUIRED_LIBRARIES dbghelp)
check_cxx_symbol_exists (UnDecorateSymbolName "windows.h;dbghelp.h" HAVE_DBGHELP)
Expand Down
4 changes: 0 additions & 4 deletions bazel/glog.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ def glog_library(with_gflags = 1, **kwargs):
"-Wno-unused-function",
"-Wno-unused-local-typedefs",
"-Wno-unused-variable",
# Allows to include pthread.h.
"-DHAVE_PTHREAD",
# Allows src/logging.cc to determine the host name.
"-DHAVE_SYS_UTSNAME_H",
# For src/utilities.cc.
Expand Down Expand Up @@ -98,8 +96,6 @@ def glog_library(with_gflags = 1, **kwargs):
darwin_only_copts = [
# For stacktrace.
"-DHAVE_DLADDR",
# Avoid deprecated syscall().
"-DHAVE_PTHREAD_THREADID_NP",
]

windows_only_copts = [
Expand Down
13 changes: 0 additions & 13 deletions src/config.h.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@
/* Define to 1 if you have the <glob.h> header file. */
#cmakedefine HAVE_GLOB_H

/* Define to 1 if you have the `pthread' library (-lpthread). */
#cmakedefine HAVE_LIBPTHREAD

/* define if you have google gmock library */
#cmakedefine HAVE_LIB_GMOCK

Expand All @@ -37,9 +34,6 @@
/* Define if you have the 'pread' function */
#cmakedefine HAVE_PREAD

/* Define if you have POSIX threads libraries and header files. */
#cmakedefine HAVE_PTHREAD

/* Define to 1 if you have the <pwd.h> header file. */
#cmakedefine HAVE_PWD_H

Expand Down Expand Up @@ -115,10 +109,6 @@
/* define if we should print file offsets in traces instead of symbolizing. */
#cmakedefine PRINT_UNSYMBOLIZED_STACK_TRACES

/* Define to necessary symbol if this constant uses a non-standard name on
your system. */
#cmakedefine PTHREAD_CREATE_JOINABLE

/* The size of `void *', as computed by sizeof. */
#cmakedefine SIZEOF_VOID_P ${SIZEOF_VOID_P}

Expand All @@ -128,7 +118,4 @@
/* Define if thread-local storage is enabled. */
#cmakedefine GLOG_THREAD_LOCAL_STORAGE

/* Replacement for deprecated syscall(SYS_gettid) on macOS. */
#cmakedefine HAVE_PTHREAD_THREADID_NP ${HAVE_PTHREAD_THREADID_NP}

#endif // GLOG_CONFIG_H
6 changes: 4 additions & 2 deletions src/glog/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <ostream>
#include <sstream>
#include <string>
#include <thread>
#include <utility>
#include <vector>

Expand Down Expand Up @@ -130,7 +131,8 @@ struct GLOG_EXPORT LogMessageTime {
struct LogMessageInfo {
explicit LogMessageInfo(const char* const severity_,
const char* const filename_, const int& line_number_,
const int& thread_id_, const LogMessageTime& time_)
std::thread::id thread_id_,
const LogMessageTime& time_)
: severity(severity_),
filename(filename_),
line_number(line_number_),
Expand All @@ -140,7 +142,7 @@ struct LogMessageInfo {
const char* const severity;
const char* const filename;
const int& line_number;
const int& thread_id;
std::thread::id thread_id;
const LogMessageTime& time;
};

Expand Down
53 changes: 0 additions & 53 deletions src/googletest.h
Original file line number Diff line number Diff line change
Expand Up @@ -579,59 +579,6 @@ struct FlagSaver {
};
#endif

class Thread {
public:
virtual ~Thread() = default;

void SetJoinable(bool) {}
#if defined(GLOG_OS_WINDOWS) && !defined(GLOG_OS_CYGWIN)
void Start() {
handle_ = CreateThread(nullptr, 0, &Thread::InvokeThreadW, this, 0, &th_);
CHECK(handle_) << "CreateThread";
}
void Join() { WaitForSingleObject(handle_, INFINITE); }
#elif defined(HAVE_PTHREAD)
void Start() { pthread_create(&th_, nullptr, &Thread::InvokeThread, this); }
void Join() { pthread_join(th_, nullptr); }
#else
void Start() {}
void Join() {}
#endif

protected:
virtual void Run() = 0;

private:
static void* InvokeThread(void* self) {
(static_cast<Thread*>(self))->Run();
return nullptr;
}

#if defined(GLOG_OS_WINDOWS) && !defined(GLOG_OS_CYGWIN)
static DWORD __stdcall InvokeThreadW(LPVOID self) {
InvokeThread(self);
return 0;
}
HANDLE handle_;
DWORD th_;
#elif defined(HAVE_PTHREAD)
pthread_t th_;
#endif
};

static inline void SleepForMilliseconds(unsigned t) {
#ifndef GLOG_OS_WINDOWS
# if defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE >= 199309L
const struct timespec req = {0, t * 1000 * 1000};
nanosleep(&req, nullptr);
# else
usleep(t * 1000);
# endif
#else
Sleep(t);
#endif
}

// Add hook for operator new to ensure there are no memory allocation.

void (*g_new_hook)() = nullptr;
Expand Down
12 changes: 7 additions & 5 deletions src/logging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <mutex>
#include <shared_mutex>
#include <string>
#include <thread>

#include "base/commandlineflags.h" // to get the program name
#include "base/googleinit.h"
Expand Down Expand Up @@ -1685,14 +1686,14 @@ void LogMessage::Init(const char* file, int line, LogSeverity severity,
<< logmsgtime_.day() << ' ' << setw(2) << logmsgtime_.hour()
<< ':' << setw(2) << logmsgtime_.min() << ':' << setw(2)
<< logmsgtime_.sec() << "." << setw(6) << logmsgtime_.usec()
<< ' ' << setfill(' ') << setw(5)
<< static_cast<unsigned int>(GetTID()) << setfill('0') << ' '
<< data_->basename_ << ':' << data_->line_ << "] ";
<< ' ' << setfill(' ') << setw(5) << std::this_thread::get_id()
<< setfill('0') << ' ' << data_->basename_ << ':' << data_->line_
<< "] ";
} else {
custom_prefix_callback(
stream(),
LogMessageInfo(LogSeverityNames[severity], data_->basename_,
data_->line_, GetTID(), logmsgtime_),
data_->line_, std::this_thread::get_id(), logmsgtime_),
custom_prefix_callback_data);
stream() << " ";
}
Expand Down Expand Up @@ -2124,7 +2125,8 @@ string LogSink::ToString(LogSeverity severity, const char* file, int line,
<< ' ' << setw(2) << logmsgtime.hour() << ':' << setw(2)
<< logmsgtime.min() << ':' << setw(2) << logmsgtime.sec() << '.'
<< setw(6) << logmsgtime.usec() << ' ' << setfill(' ') << setw(5)
<< GetTID() << setfill('0') << ' ' << file << ':' << line << "] ";
<< std::this_thread::get_id() << setfill('0') << ' ' << file << ':'
<< line << "] ";

// A call to `write' is enclosed in parenthneses to prevent possible macro
// expansion. On Windows, `write' could be a macro defined for portability.
Expand Down
39 changes: 22 additions & 17 deletions src/logging_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
# include <sys/wait.h>
#endif

#include <chrono>
#include <cstdio>
#include <cstdlib>
#include <fstream>
Expand All @@ -54,6 +55,7 @@
#include <queue>
#include <sstream>
#include <string>
#include <thread>
#include <vector>

#include "base/commandlineflags.h"
Expand Down Expand Up @@ -1197,12 +1199,9 @@ static vector<string> global_messages;
// helper for TestWaitingLogSink below.
// Thread that does the logic of TestWaitingLogSink
// It's free to use LOG() itself.
class TestLogSinkWriter : public Thread {
class TestLogSinkWriter {
public:
TestLogSinkWriter() {
SetJoinable(true);
Start();
}
TestLogSinkWriter() : t_{&TestLogSinkWriter::Run, this} {}

// Just buffer it (can't use LOG() here).
void Buffer(const string& message) {
Expand All @@ -1215,11 +1214,12 @@ class TestLogSinkWriter : public Thread {

// Wait for the buffer to clear (can't use LOG() here).
void Wait() {
using namespace std::chrono_literals;
RAW_LOG(INFO, "Waiting");
mutex_.lock();
while (!NoWork()) {
mutex_.unlock();
SleepForMilliseconds(1);
std::this_thread::sleep_for(1ms);
mutex_.lock();
}
RAW_LOG(INFO, "Waited");
Expand All @@ -1232,6 +1232,8 @@ class TestLogSinkWriter : public Thread {
should_exit_ = true;
}

void Join() { t_.join(); }

private:
// helpers ---------------

Expand All @@ -1240,12 +1242,13 @@ class TestLogSinkWriter : public Thread {
bool HaveWork() { return !messages_.empty() || should_exit_; }

// Thread body; CAN use LOG() here!
void Run() override {
void Run() {
using namespace std::chrono_literals;
while (true) {
mutex_.lock();
while (!HaveWork()) {
mutex_.unlock();
SleepForMilliseconds(1);
std::this_thread::sleep_for(1ms);
mutex_.lock();
}
if (should_exit_ && messages_.empty()) {
Expand All @@ -1255,7 +1258,7 @@ class TestLogSinkWriter : public Thread {
// Give the main thread time to log its message,
// so that we get a reliable log capture to compare to golden file.
// Same for the other sleep below.
SleepForMilliseconds(20);
std::this_thread::sleep_for(20ms);
RAW_LOG(INFO, "Sink got a messages"); // only RAW_LOG under mutex_ here
string message = messages_.front();
messages_.pop();
Expand All @@ -1264,7 +1267,7 @@ class TestLogSinkWriter : public Thread {
// e.g. pushing the message over with an RPC:
size_t messages_left = messages_.size();
mutex_.unlock();
SleepForMilliseconds(20);
std::this_thread::sleep_for(20ms);
// May not use LOG while holding mutex_, because Buffer()
// acquires mutex_, and Buffer is called from LOG(),
// which has its own internal mutex:
Expand All @@ -1277,6 +1280,7 @@ class TestLogSinkWriter : public Thread {

// data ---------------

std::thread t_;
std::mutex mutex_;
bool should_exit_{false};
queue<string> messages_; // messages to be logged
Expand All @@ -1288,7 +1292,7 @@ class TestLogSinkWriter : public Thread {
class TestWaitingLogSink : public LogSink {
public:
TestWaitingLogSink() {
tid_ = pthread_self(); // for thread-specific behavior
tid_ = std::this_thread::get_id(); // for thread-specific behavior
AddLogSink(this);
}
~TestWaitingLogSink() override {
Expand All @@ -1306,19 +1310,19 @@ class TestWaitingLogSink : public LogSink {
// Push it to Writer thread if we are the original logging thread.
// Note: Something like ThreadLocalLogSink is a better choice
// to do thread-specific LogSink logic for real.
if (pthread_equal(tid_, pthread_self())) {
if (tid_ == std::this_thread::get_id()) {
writer_.Buffer(ToString(severity, base_filename, line, logmsgtime,
message, message_len));
}
}

void WaitTillSent() override {
// Wait for Writer thread if we are the original logging thread.
if (pthread_equal(tid_, pthread_self())) writer_.Wait();
if (tid_ == std::this_thread::get_id()) writer_.Wait();
}

private:
pthread_t tid_;
std::thread::id tid_;
TestLogSinkWriter writer_;
};

Expand All @@ -1329,15 +1333,16 @@ static void TestLogSinkWaitTillSent() {
// reentered
global_messages.clear();
{
using namespace std::chrono_literals;
TestWaitingLogSink sink;
// Sleeps give the sink threads time to do all their work,
// so that we get a reliable log capture to compare to the golden file.
LOG(INFO) << "Message 1";
SleepForMilliseconds(60);
std::this_thread::sleep_for(60ms);
LOG(ERROR) << "Message 2";
SleepForMilliseconds(60);
std::this_thread::sleep_for(60ms);
LOG(WARNING) << "Message 3";
SleepForMilliseconds(60);
std::this_thread::sleep_for(60ms);
}
for (auto& global_message : global_messages) {
LOG(INFO) << "Sink capture: " << global_message;
Expand Down
Loading

0 comments on commit 066300c

Please sign in to comment.