Crash less #8
Open
Crash less #8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I'm not sure if we can guarantee these addresses to be consistent with the actual file descriptors, this should ensure that the FD value and the hash key are indeed equivalent.
We now use the same pattern for timeouts and inputs, and seemingly prevent quite a lot of crashes around input_remove. Also some usual cosmetic touchups.
Half-Shot
approved these changes
Sep 15, 2021
| s_evLoopInputEvent *inputEvent = handle; | ||
| s_evLoopInput *input = inputEvent->parent; | ||
| // Why is this here? XXX | ||
| if (g_list_find(input->events, inputEvent) == NULL) { |
There was a problem hiding this comment.
I think this is used to check if the input actually exists in our hash, but granted it's probably pointless here.
purple needs a guint ID, which is 32bit. 64bit pointers would get trimmed, and would later blow up when converted back to the pointers, as we'd get a different value than we actually allocated and initialized. This switches it to a lookup table with meaningless integers for the IDs.
tadzik
commented
Sep 15, 2021
| input_event->parent = input_handle; | ||
| input_handle->events = g_list_append(input_handle->events, input_event); | ||
|
|
||
| input_event->id = evLoopState.last_event_id++; |
There was a problem hiding this comment.
Should probably check if we didn't overflow here – how knows for how many decades it can run now that it got as stable as it is ;)
There was a problem hiding this comment.
we can check if evLoopState.last_event_id is maxint if you like, but honestly...meeeh
|
FYI the last commit caused lots of crashes on bifrost, I had to roll back by one. |
Can't reproduce :/ Would it be possible to get a core dump if you're still getting it? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An assortment of fixes which seem to significantly improve stability – though it was never predictable or reproducible, so I may have just gotten lucky with the results here. They do seem to make sense though.
I've seen a crash on startup still happening a few times with input_remove getting a completely bogus handle as an argument according to gdb, so I left the logs in (as
g_warningso that they're always on) in case it happens again, to give us something to work with.