Skip to content

Commit

Permalink
Fix crash related to unwind protect short circuiting
Browse files Browse the repository at this point in the history
It seems relying on non-local static objects can cause memory issues,
that are resolved by moving to use a local static objects instead.

Fixes #244
  • Loading branch information
jimhester committed Nov 1, 2021
1 parent c22dc9b commit 9a62c3a
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# cpp11 (development version)

* Fix crash related to unwind protect optimization (#244)

# cpp11 0.4.0

## New Features
Expand Down
11 changes: 5 additions & 6 deletions inst/include/cpp11/protect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ inline Rboolean& get_should_unwind_protect() {
return should_unwind_protect[0];
}

static Rboolean& should_unwind_protect = get_should_unwind_protect();

} // namespace detail

#ifdef HAS_UNWIND_PROTECT
Expand All @@ -82,11 +80,12 @@ static Rboolean& should_unwind_protect = get_should_unwind_protect();
template <typename Fun, typename = typename std::enable_if<std::is_same<
decltype(std::declval<Fun&&>()()), SEXP>::value>::type>
SEXP unwind_protect(Fun&& code) {
if (detail::should_unwind_protect == FALSE) {
static auto should_unwind_protect = detail::get_should_unwind_protect();

This comment has been minimized.

Copy link
@jimhester

jimhester Nov 16, 2022

Author Member

@DavisVaughan did you try changing this to static auto& should_unwind_protect? It is possible that the type deduction is not using a reference, so the subsequent assignment is assigning to a copy rather than a reference as intended.

If this fixes the issue it would be a simpler change than that proposed in #298

if (should_unwind_protect == FALSE) {
return std::forward<Fun>(code)();
}

detail::should_unwind_protect = FALSE;
should_unwind_protect = FALSE;

static SEXP token = [] {
SEXP res = R_MakeUnwindCont();
Expand All @@ -96,7 +95,7 @@ SEXP unwind_protect(Fun&& code) {

std::jmp_buf jmpbuf;
if (setjmp(jmpbuf)) {
detail::should_unwind_protect = TRUE;
should_unwind_protect = TRUE;
throw unwind_exception(token);
}

Expand All @@ -121,7 +120,7 @@ SEXP unwind_protect(Fun&& code) {
// unset it here before returning the value ourselves.
SETCAR(token, R_NilValue);

detail::should_unwind_protect = TRUE;
should_unwind_protect = TRUE;

return res;
}
Expand Down

0 comments on commit 9a62c3a

Please sign in to comment.