mirrors/lemmy
1
0
Fork 0
mirror of https://github.com/LemmyNet/lemmy.git synced 2025-09-02 19:23:49 +00:00
Code Issues Projects Releases Wiki Activity

Prevent stack overflow when fetching nested comment (#5787)

Browse source 
* attempt future wrapper

* Revert "attempt future wrapper"

This reverts commit ce95422228.

* use spawn

* remove `lazy` and change comment

* temporary change for test

* change 5000 back to 50

* fix comment about async laziness
This commit is contained in:
dullbananas 2025-07-22 12:37:50 -07:00 committed by GitHub
parent 6e292141cc
commit b3e5e6c76a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 21 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
crates/apub_objects/src/protocol/note.rs
View file

@ -70,16 +70,30 @@ impl Note {
) -> LemmyResult<(ApubPost, Option<ApubComment>)> {
// We use recursion here to fetch the entire comment chain up to the top-level parent. This is
// necessary because we need to know the post and parent comment in order to insert a new
// comment. However it can also lead to stack overflow when fetching many comments recursively.
// To avoid this we check the request count against max comment depth, which based on testing
// can be handled without risking stack overflow. This is not a perfect solution, because in
// some cases we have to fetch user profiles too, and reach the limit after only 25 comments
// or so.
// A cleaner solution would be converting the recursion into a loop, but that is tricky.
// comment. However it can also lead to too much resource consumption when fetching many
// comments recursively. To avoid this we check the request count against max comment depth.
//
// A separate task is spawned for the recursive call. Otherwise, when the async executor polls
// the task this is in, the poll function's call stack would grow with the level of recursion,
// so a stack overflow would be possible.
//
// The stack overflow prevention relies on the total laziness that the async keyword provides
// (https://rust-lang.github.io/rfcs/2394-async_await.html#async-functions). This means you need
// to be careful if you want to change `Note::get_parents` and `CreateOrUpdateNote::verify` from
// `async fn foo(...) -> T` to `fn foo(...) -> impl Future<Output = T>`. Between each level of
// recursion, there must be the beginning of at least one `async` block or `async fn`,
// otherwise there might be multiple levels of recursion before the first poll.
if context.request_count() > MAX_COMMENT_DEPTH_LIMIT.try_into()? {
Err(LemmyErrorType::MaxCommentDepthReached)?;
}
let parent = self.in_reply_to.dereference(context).await?;
let parent = tokio::spawn({
let in_reply_to = self.in_reply_to.clone();
let context = context.clone();
// This is wrapped in an async block only to satisfy the borrow checker. This wrapping is not
// needed for the stack overflow prevention.
async move { in_reply_to.dereference(&context).await }
})
.await??;
match parent {
PostOrComment::Left(p) => Ok((p.clone(), None)),
PostOrComment::Right(c) => {