Skip to content

Conversation

@vraychev
Copy link
Owner

No description provided.

kekstee and others added 30 commits March 4, 2016 08:55
Avoid overriding the configured docdir inside the Makefile.
$(datarootdir)/doc/ may differ from $(prefix)/share/doc/
See https://code.google.com/p/google-glog/issues/detail?id=209 - this is an updated version of that patch.

It adds a flag that allows to switch behavior from base_filename + filename_extension + time_pid_string to base_filename + filename_extension, while still defaulting to the current behavior to avoid breakage in existing code. This change would allow easier log rotation schemes and better control on what's written on disk.
Previously, the implementation of google::GetStackTrace() that uses
libunwind uses a global variable that enforces that only one thread may
invoke libunwind at a time. However, libunwind is thread-safe. The
comment above the variable indicates that it is to protect against
reentrancy issues.

Instead of using a global variable, it would be much better to use a
thread-local variable to protect against these reentrancy issues. That
should provide the needed reentrancy protection while allowing multiple
threads to get stack traces at the same time.

It also allows for the removal of the atomic CAS operations on the
variable.

Resolves #160.
Summary:
Issue #80 points out several places in glog where TSAN discovers
false positives. One of these places is in the `LOG_EVERY_N` macros.
These macros are implemented by maintaining a static unprotected
integer counter, and TSAN will report data races on these counters.

Here is a minimum example to reproduce the data race:

```

void logging() {
  for (int i = 0; i < 300; ++i) {
    LOG_EVERY_N(INFO, 2) << "foo";
  }
}

int main() {
  auto t1 = std::thread(logging);
  auto t2 = std::thread(logging);
  t1.join();
  t2.join();
  return 0;
}
```

And here is the TSAN report:

```
WARNING: ThreadSanitizer: data race (pid=776850)
  Write of size 4 at 0x558de483f684 by thread T2:
    #0 logging()
    #1 void std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>)
    #2 std::_Bind_simple<void (*())()>::operator()()
    #3 std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run()
    #4 execute_native_thread_routine

  Previous write of size 4 at 0x558de483f684 by thread T1:
    #0 logging()
    #1 void std::_Bind_simple<void (*())()>::_M_invoke<>(std::_Index_tuple<>)
    #2 std::_Bind_simple<void (*())()>::operator()()
    #3 std::thread::_Impl<std::_Bind_simple<void (*())()> >::_M_run()
    #4 execute_native_thread_routine

  Location is global '<null>' at 0x000000000000 (main+0x00000011c684)

  Thread T2 (tid=776857, running) created by main thread at:
    #0 pthread_create
    #1 __gthread_create
    #2 std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>, void (*)())
    #3 main

  Thread T1 (tid=776856, running) created by main thread at:
    #0 pthread_create
    #1 __gthread_create
    #2 std::thread::_M_start_thread(std::shared_ptr<std::thread::_Impl_base>, void (*)())
    #3 main

SUMMARY: ThreadSanitizer: data race in logging()
```

To avoid noisy TSAN reports and also avoid adding a performance hit, this
change will mark these counters as benign races so that TSAN will not report
them. This change will only have an effect if we are compiling with TSAN;
there are no changes if we are not building with TSAN.

With this change, the above example no longer reports a data race when built
and run with TSAN.
If the getpwuid_r method doesn't find an entry with the given ID, it
will still return success (0), but the *result will be set to NULL.
This checks the |result| value so it won't crash if it doesn't find
the entry.

This normally shouldn't ever happen, but it can somehow happen on
iOS simulators.
Fixed these warnings:

src/logging_unittest.cc: At global scope:
src/logging_unittest.cc:1081:13: warning: 'void MyCheck(bool, bool)' defined but not used [-Wunused-function]
 static void MyCheck(bool a, bool b) {
             ^~~~~~~
src/logging_unittest.cc:1078:13: warning: 'void MyFatal()' defined but not used [-Wunused-function]
 static void MyFatal() {
             ^~~~~~~
When glog 0.4.0 is built using musl instead of glibc, the compilation
for this test fails because musl doesn't provide execinfo.h, which sets
HAVE_STACKTRACE to false.

Tested by building glog with musl and verifying that the build succeeds
with this patch. [See openwrt/packages#8583]

Signed-off-by: Amol Bhave <ambhave@fb.com>
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
src/symbolize.cc: fix build without dlfcn.h
usleep is deprecated and optionally not available with uClibc-ng.
uClibc++ has compiler support for C++11, but not the functions.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
googletest: Switch to nanosleep
logging: Fix compilation with uClibc++
Fix symbolize_unittest for musl builds
Fix build for Android API < 21
Use thread local for libunwind GetStackTrace() reentrancy protection
Respect configured docdir
Allow getpwuid_r to return missing entry.
Added fixed log name support
Extend the LogSink interface to be able to pass microseconds
* Add support for automatic removal of old logs

GetOverdueLogNames(string log_directory, int days) will check all
filenames under log_directory, and return a list of files whose last modified time is
over the given days (calculated using difftime()).

So that we can easily for unlink all files stored in the returned vector.

* Replaced the lines that require C++11

* embed dirent.h in project

* Add support for automatic removal of old logs

In this commit, at the end of LogFileObject::Write,
it will perform clean up for old logs.

It uses GetLoggingDirectories() and for each file in each
directory, it will check if a file is a log file produced by glog.
If it is, and it is last modified 3 days ago, then it will unlink()
this file. (It will only remove the project's own log files,
it won't remove the logs from other projects.)

Currently it is hardcoded to 3 days, I'll see if this can be
implemented in a more flexible manner.

* Implement old log cleaner

The log cleaner can be enabled and disabled at any given time.
By default, the log cleaner is disabled.

For example, this will enable the log cleaner and delete
the log files whose last modified time is >= x days
google::EnableLogCleaner(x days);

To disable it, simply call
google::DisableLogCleaner();

Please note that it will only clean up the logs produced for
its own project, the log files from other project will be untouched.

* logging: log_cleaner: Use blackslash for windows dir delim

* logging: log_cleaner: remove the range-based loops

Also replaced the hardcoded overdue days with the correct variable.

* Add Marco Wang to AUTHORS and CONTRIBUTORS

* logging: log_cleaner: Remove redundant filename stripping

Previously the full path to a file is passed into IsGlogLog(),
and then std::string::erase() is used to get the filename part.
If a directory name contains '.', then this function will be unreliable.

Now only the filename it self is passed into IsGlogLog(),
so this problem will be eradicated.

* logging: log_cleaner: improve readability

* Add google::EnableLogCleaner() to windows logging.h

* logging: log_cleaner: Remove perror message

* logging: IsGlogLog: match filename keyword by keyword

Splitting a filename into tokens by '.' causes problems
if the executable's filename contains a dot.

Filename should be matched keyword by keyword in the following
order:
1. program name
2. hostname
3. username
4. "log"
Revert "Added fixed log name support"
StephLin and others added 28 commits June 19, 2021 17:28
Ubuntu 16.04 is end-of-life, we're going to remove it from Bazel CI.

If you like you can add testing on `ubuntu2004` platform which we also support.
Use <chrono> and <atomic> for C++11 or greater.
For non-Windows pre-C++11 systems, use <time.h> and built-in atomic operations.
For Windows pre-C++11, use the Windows implementations for time and atomic operations.
log messages periodically (time-based)
cmake: prefer linking against gflags::gflags (fixes #683)
Fix syscall deprecation warning on macOS >= 10.12
fixed exception specification mismatch
Account for ABI incompatibility.
Add a static cast to compare unsigned with unsigned
Fix: not implement virtual class when WITH_CUSTOM_PREFIX on
This makes this definition consistent with Abseil's declarations.  See:
https://github.com/abseil/abseil-cpp/blob/master/absl/base/dynamic_annotations.h#L213-L215

Having this unsigned seems to match what libsanitizers is expecting:
https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libsanitizer/tsan/tsan_interface_ann.cpp#L367-L371

This solves a "conflicting declaration of C function" when using glog
with abseil.
Change size type in AnnotateBenignRaceSized to size_t from long
}

for (size_t idx = 0; idx < LogTimes::MAX_CALLS - 1; ++idx) {
int64 time_ns = nsBetweenCalls[idx];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UNINITIALIZED_VALUE: The value read from nsBetweenCalls[_] was never initialized.
(at-me in a reply with help or ignore)

@vraychev vraychev merged commit c10d4bc into vraychev:master Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.