Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix endless loop on crash after init during +load #338

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix endless loop on crash after init during +load #338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33247f1
refactor init of shared_handler_context to force correct order during…
cynicer Jan 16, 2025
462bd10
update changelog
cynicer Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* **[Improvement]** Update target iOS and tvOS version to 12.0.
* **[Improvement]** Support Xcode 16 build.
* **[Fix]** Fix endless loop when PLCrashReporter was set up during `load`.

___

Expand Down
53 changes: 29 additions & 24 deletions Source/PLCrashSignalHandler.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@
* Signal handler context that must be global for async-safe
* access.
*/
static struct {
struct shared_handler_context {
/** @internal
* Registered callbacks. */
async_list<plcrash_signal_user_callback> callbacks;
Expand All @@ -97,7 +97,12 @@
* Originaly registered signal handlers. This list should only be mutated in
* -[PLCrashSignalHandler registerHandlerWithSignal:error:] with the appropriate locks held. */
async_list<plcrash_signal_handler_action> previous_actions;
} shared_handler_context;
};

shared_handler_context& shared_handler_context() {
static struct shared_handler_context instance;
return instance;
}

/*
* Finds and executes the first matching signal handler in the shared previous_actions list; this is used
Expand All @@ -111,10 +116,10 @@ static bool previous_action_callback (int signo, siginfo_t *info, ucontext_t *ua
if (PLCrashSignalHandlerForward(nextHandler, signo, info, uap))
return true;

shared_handler_context.previous_actions.set_reading(true); {
shared_handler_context().previous_actions.set_reading(true); {
/* Find the first matching handler */
async_list<plcrash_signal_handler_action>::node *next = NULL;
while ((next = shared_handler_context.previous_actions.next(next)) != NULL) {
while ((next = shared_handler_context().previous_actions.next(next)) != NULL) {
/* Skip non-matching entries */
if (next->value().signo != signo)
continue;
Expand Down Expand Up @@ -145,7 +150,7 @@ static bool previous_action_callback (int signo, siginfo_t *info, ucontext_t *ua
/* Handler was found; iteration done */
break;
}
} shared_handler_context.previous_actions.set_reading(false);
} shared_handler_context().previous_actions.set_reading(false);

return handled;
}
Expand All @@ -158,18 +163,18 @@ static bool internal_callback_iterator (int signo, siginfo_t *info, ucontext_t *
/* Call the next handler in the chain. If this is the last handler in the chain, pass it the original signal
* handlers. */
bool handled = false;
shared_handler_context.callbacks.set_reading(true); {
shared_handler_context().callbacks.set_reading(true); {
async_list<plcrash_signal_user_callback>::node *prev = (async_list<plcrash_signal_user_callback>::node *) context;
async_list<plcrash_signal_user_callback>::node *current = shared_handler_context.callbacks.next(prev);
async_list<plcrash_signal_user_callback>::node *current = shared_handler_context().callbacks.next(prev);

/* Check for end-of-list */
if (current == NULL) {
shared_handler_context.callbacks.set_reading(false);
shared_handler_context().callbacks.set_reading(false);
return false;
}

/* Check if any additional handlers are registered. If so, provide the next handler as the forwarding target. */
if (shared_handler_context.callbacks.next(current) != NULL) {
if (shared_handler_context().callbacks.next(current) != NULL) {
PLCrashSignalHandlerCallback next_handler = {
.callback = internal_callback_iterator,
.context = current
Expand All @@ -179,7 +184,7 @@ static bool internal_callback_iterator (int signo, siginfo_t *info, ucontext_t *
/* Otherwise, we've hit the final handler in the list. */
handled = current->value().callback(signo, info, uap, current->value().context, NULL);
}
} shared_handler_context.callbacks.set_reading(false);
} shared_handler_context().callbacks.set_reading(false);

return handled;
};
Expand Down Expand Up @@ -263,18 +268,18 @@ + (PLCrashSignalHandler *) sharedHandler {
*/
+ (void) resetHandlers {
/* Reset all saved signal handlers */
shared_handler_context.previous_actions.set_reading(true); {
shared_handler_context().previous_actions.set_reading(true); {
async_list<plcrash_signal_handler_action>::node *next = NULL;
while ((next = shared_handler_context.previous_actions.next(next)) != NULL)
shared_handler_context.previous_actions.nasync_remove_node(next);
} shared_handler_context.previous_actions.set_reading(false);
while ((next = shared_handler_context().previous_actions.next(next)) != NULL)
shared_handler_context().previous_actions.nasync_remove_node(next);
} shared_handler_context().previous_actions.set_reading(false);

/* Reset all callbacks */
shared_handler_context.callbacks.set_reading(true); {
shared_handler_context().callbacks.set_reading(true); {
async_list<plcrash_signal_user_callback>::node *next = NULL;
while ((next = shared_handler_context.callbacks.next(next)) != NULL)
shared_handler_context.callbacks.nasync_remove_node(next);
} shared_handler_context.callbacks.set_reading(false);
while ((next = shared_handler_context().callbacks.next(next)) != NULL)
shared_handler_context().callbacks.nasync_remove_node(next);
} shared_handler_context().callbacks.set_reading(false);
}

/**
Expand Down Expand Up @@ -344,21 +349,21 @@ - (BOOL) registerHandlerWithSignal: (int) signo error: (NSError **) outError {
.callback = previous_action_callback,
.context = NULL
};
shared_handler_context.callbacks.nasync_append(sa);
shared_handler_context().callbacks.nasync_append(sa);
}

/* Check whether the signal already has a registered handler. */
BOOL isRegistered = NO;
shared_handler_context.previous_actions.set_reading(true); {
shared_handler_context().previous_actions.set_reading(true); {
/* Find the first matching handler */
async_list<plcrash_signal_handler_action>::node *next = NULL;
while ((next = shared_handler_context.previous_actions.next(next)) != NULL) {
while ((next = shared_handler_context().previous_actions.next(next)) != NULL) {
if (next->value().signo == signo) {
isRegistered = YES;
break;
}
}
} shared_handler_context.previous_actions.set_reading(false);
} shared_handler_context().previous_actions.set_reading(false);

/* Register handler for the requested signal */
if (!isRegistered) {
Expand Down Expand Up @@ -389,7 +394,7 @@ - (BOOL) registerHandlerWithSignal: (int) signo error: (NSError **) outError {
.signo = signo,
.action = sa_prev
};
shared_handler_context.previous_actions.nasync_append(act);
shared_handler_context().previous_actions.nasync_append(act);
}
} pthread_mutex_unlock(&registerHandlers);

Expand Down Expand Up @@ -426,7 +431,7 @@ - (BOOL) registerHandlerForSignal: (int) signo
.callback = callback,
.context = context
};
shared_handler_context.callbacks.nasync_prepend(reg);
shared_handler_context().callbacks.nasync_prepend(reg);

return YES;
}
Expand Down