SocketPool: Initially allocate these as long-lived objects #5235
Conversation
This file contains 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 cannot give a satisfactory account of _why_, but the crash in adafruit#5021 goes away when this object is initially allocated in the long-lived portion of the heap. Closes: adafruit#5021
tannewt
requested changes
Aug 26, 2021
| @@ -47,7 +47,7 @@ | |||
| STATIC mp_obj_t socketpool_socketpool_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *args, mp_map_t *kw_args) { | |||
| mp_arg_check_num(n_args, kw_args, 1, 1, false); | |||
|
|
|||
| socketpool_socketpool_obj_t *s = m_new_obj_with_finaliser(socketpool_socketpool_obj_t); | |||
| socketpool_socketpool_obj_t *s = m_new_ll_obj_with_finaliser(socketpool_socketpool_obj_t); | |||
There was a problem hiding this comment.
I don't see a finaliser for it. Maybe that is the issue?
I'd like to understand our fix before we merge something. This feels similar to my "allocate less long lived and the problem goes away."
|
Now it makes sense WHY this change could affect anything (the socketpool was the long-lived object at block 1 of the GC heap) but this is not the right fix. we all knew that. |
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.
I cannot give a satisfactory account of why, but the crash in #5021 goes away when this object is initially allocated in the long-lived portion of the heap.
Closes: #5021