Skip to content

Pin ChannelHandle to permanent memory address and use pointers#4675

Closed
uklotzde wants to merge 3 commits intomixxxdj:mainfrom
uklotzde:channelhandle
Closed

Pin ChannelHandle to permanent memory address and use pointers#4675
uklotzde wants to merge 3 commits intomixxxdj:mainfrom
uklotzde:channelhandle

Conversation

@uklotzde
Copy link
Copy Markdown
Contributor

@uklotzde uklotzde commented Feb 12, 2022

Have fun.

Disclaimer: I didn't write the code originally and I won't rewrite it. Take it as is or leave it. I am not answering any questions about design decisions and will ignore any nitpicking comments. Otherwise, cherry-pick my commit and rework it as you like. But don't expect me to review your code. Update: I am well aware that the code could still be optimized. But that is a rabbit hole that I prefer to avoid.

2.3 is also affected and might stop working at any time. This issue could even be the cause for the spurious std::bad_alloc crashes that some users have reported.

@uklotzde uklotzde mentioned this pull request Feb 12, 2022
@daschuer
Copy link
Copy Markdown
Member

Please explain how the code fixes the issue.
I do not understand why we need the address of the handler at all.
For my understanding the ChannelHandle is just an int index of m_groupToHandle. I do not understand why we use the address of the handler at all. It should be a hidden implementation details.

@uklotzde
Copy link
Copy Markdown
Contributor Author

Please explain how the code fixes the issue. I do not understand why we need the address of the handler at all. For my understanding the ChannelHandle is just an int index of m_groupToHandle. I do not understand why we use the address of the handler at all. It should be a hidden implementation details.

If you have other ideas please provide a fix. Nothing more to say, please read my disclaimer.

The commit message concisely explains what has changed.

@ywwg
Copy link
Copy Markdown
Member

ywwg commented Feb 12, 2022

it's pretty shitty to say "this is the fix i refuse to explain it or entertain any questions or review comments." I agree with @daschuer, it doesn't make sense to me why these are pointers and I'd prefer to take my approach and convert them to values. We can work around the CLEAR_STRUCT issue by writing some initializers.

@uklotzde
Copy link
Copy Markdown
Contributor Author

Over and out.

@uklotzde uklotzde closed this Feb 12, 2022
@uklotzde uklotzde deleted the channelhandle branch February 13, 2022 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants