Skip to content

Commit

Permalink
Fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bernardnormier committed Nov 11, 2024
1 parent a25d0d3 commit 6601dac
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 14 deletions.
3 changes: 1 addition & 2 deletions cpp/include/Ice/Exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,8 @@ namespace Ice
/**
* Enables the collection of stack traces for exceptions. On Windows, calling this function more than once is
* useful to refresh the symbol module list; on other platforms, the second and subsequent calls have no effect.
* @return True if stack trace collection was successfully enabled; otherwise, false.
*/
static bool ice_enableStackTraceCollection();
static void ice_enableStackTraceCollection();

private:
friend ICE_API std::ostream& operator<<(std::ostream&, const Exception&);
Expand Down
9 changes: 2 additions & 7 deletions cpp/src/Ice/Exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ namespace

inline bool collectStackTraces() noexcept
{
lock_guard<mutex> lock(globalMutex);
#if defined(ICE_DBGHELP)
return process != nullptr;
#elif defined(ICE_LIBBACKTRACE)
Expand Down Expand Up @@ -466,7 +467,7 @@ Ice::Exception::ice_stackTrace() const
return getStackTrace(*_stackFrames);
}

bool
void
Ice::Exception::ice_enableStackTraceCollection()
{
lock_guard lock(globalMutex);
Expand Down Expand Up @@ -508,16 +509,10 @@ Ice::Exception::ice_enableStackTraceCollection()
{
// Leaked, as libbacktrace does not provide an API to free this state.
bstate = backtrace_create_state(0, 1, ignoreErrorCallback, 0);

// The first call to backtrace_pcinfo does not initialize bstate->fileline_fn
// in a thread-safe manner, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81098.
// So we make a "dummy" call to backtrace_pcinfo to initialize it here.
backtrace_pcinfo(bstate, 0, ignoreFrame, ignoreErrorCallback, 0);
}
#elif defined(ICE_BACKTRACE)
backTraceEnabled = true;
#endif
return collectStackTraces();
}

ostream&
Expand Down
8 changes: 3 additions & 5 deletions cpp/test/IceUtil/stacktrace/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,9 @@ class Client final : public Test::TestHelper
void
Client::run(int, char*[])
{
if (!Ice::Exception::ice_enableStackTraceCollection())
{
cout << "This Ice build cannot capture stack traces" << endl;
return;
}
Ice::Exception::ice_enableStackTraceCollection();

// We assume this test is executed only on platforms/builds with support for stack trace collection.

cout << "checking stacktrace... ";

Expand Down

0 comments on commit 6601dac

Please sign in to comment.