Skip to content

Commit

Permalink
fix: dont call through to NSLog from profiler sampling thread (#3390)
Browse files Browse the repository at this point in the history
Let's just avoid calling through to NSLog from the profiler's sampling thread until we can get a different logging solution in place, if we decide we need these logs at some point in the future.
  • Loading branch information
armcknight authored Nov 9, 2023
1 parent 11b2ffa commit 734d507
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Crash when logging from certain profiling contexts (#3390)

## 8.15.1

### Fixes
Expand Down
6 changes: 5 additions & 1 deletion Sources/Sentry/SentryProfilingLogging.mm
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "SentryProfilingLogging.hpp"

#import "SentryLog.h"
#if !defined(DEBUG)

# import "SentryLog.h"

namespace sentry {
namespace profiling {
Expand Down Expand Up @@ -41,3 +43,5 @@

} // namespace profiling
} // namespace sentry

#endif // !defined(DEBUG)
37 changes: 26 additions & 11 deletions Sources/Sentry/include/SentryProfilingLogging.hpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
#pragma once

#include <cerrno>
#include <cstring>
#include <string>
#include <unistd.h>
#include <vector>
#if !defined(DEBUG)

# include <cerrno>
# include <cstring>
# include <string>
# include <unistd.h>
# include <vector>

namespace sentry {
namespace profiling {
Expand All @@ -19,12 +21,25 @@ namespace profiling {
} // namespace profiling
} // namespace sentry

#define SENTRY_PROF_LOG_DEBUG(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Debug, __VA_ARGS__)
#define SENTRY_PROF_LOG_WARN(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Warning, __VA_ARGS__)
#define SENTRY_PROF_LOG_ERROR(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Error, __VA_ARGS__)
# define SENTRY_PROF_LOG_DEBUG(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Debug, __VA_ARGS__)
# define SENTRY_PROF_LOG_WARN(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Warning, __VA_ARGS__)
# define SENTRY_PROF_LOG_ERROR(...) \
sentry::profiling::log(sentry::profiling::LogLevel::Error, __VA_ARGS__)

#else

// Don't do anything with these in production until we can get a logging solution in place that
// doesn't use NSLog. We can't use NSLog in these codepaths because it takes a lock, and if the
// profiler's sampling thread is terminated before it can release that lock, then subsequent
// attempts to acquire it can cause a crash.
// See https://github.com/getsentry/sentry-cocoa/issues/3336#issuecomment-1802892052 for more info.
# define SENTRY_PROF_LOG_DEBUG(...)
# define SENTRY_PROF_LOG_WARN(...)
# define SENTRY_PROF_LOG_ERROR(...)

#endif // !defined(DEBUG)

/**
* Logs the error code returned by executing `statement`, and returns the
Expand Down

0 comments on commit 734d507

Please sign in to comment.