3 Commits

Author SHA1 Message Date
Mark Mentovai
1abaf22e28 Use readdir() instead of readdir_r() on all (POSIX) platforms
readdir_r() is a thread-safe version of readdir(), although readdir() is
not particularly thread-unsafe with most usage. The dirent* returned by
readdir() can only be invalidated by a subsequent readdir() or
closedir() on the same DIR*. In typical usage, where a returned dirent*
is used exclusively within a loop around readdir() and is not expected
to outlive that loop, there are no lifetime or thread-safety issues with
the use of readdir().

readdir_r() may be harmful in certain situations because its buffer is
not explicitly sized, and attempts to provide a suitably sized buffer
dynamically (which, incidentally, our code did not do) are subject to a
race condition.

https://elliotth.blogspot.com/2012/10/how-not-to-use-readdirr3.html
https://womble.decadent.org.uk/readdir_r-advisory.html

glibc has already deprecated readdir_r(), and all Linux (including
Android) code was already using readdir(). This change eliminates
variant codepaths. It delegates buffer sizing (which we weren’t doing
correctly) to the C library, which also has more options at its disposal
to avoid races in sizing that buffer.

Change-Id: I4fca8948454116360180ad0017f226d06727ef81
Reviewed-on: https://chromium-review.googlesource.com/705756
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
2017-10-06 21:08:50 +00:00
Mark Mentovai
7e6a0145b1 mac handler: Record the number of open files in the handler process
The "file-limit" annotation has shown that the system as a whole is not
likely to be out of file descriptors globally. It’s possible that a file
descriptor leak in crashpad_handler itself is responsible for certain
crashes. Add a count of the number of open files in the handler process
to this annotation to test this theory.

Bug: crashpad:180
Change-Id: If6f2304fdabddd29636ba4ac5a7d1e0fff7f4b61
Reviewed-on: https://chromium-review.googlesource.com/585852
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-07-26 17:21:45 +00:00
Mark Mentovai
b8aaa22905 mac handler: Record a file-limits annotation (temporarily)
The "file-limit" annotation will be used to confirm the theory that
certain crashes are caused by systems at or near their file descriptor
table size limits.

The annotation records the system-wide kern.num_files and kern.maxfiles
values, and the process-specific current and maximum file descriptor
limits.

The annotation will be set on crashpad_handler startup, and will be
refreshed every time an exception is handled and every time the upload
thread processes a pending report.

It’s expected that this annotation will be removed after enough data has
been collected to confirm the theory. However, the principle is useful
enough that we may want to provide this feature more generally under
bugs 19 or 21.

Bug: crashpad:180
Change-Id: I3bb78fae60e0567bc4ac2625716e0abe0ddae08c
Reviewed-on: https://chromium-review.googlesource.com/479914
Reviewed-by: Robert Sesek <rsesek@chromium.org>
2017-04-18 17:27:31 +00:00