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.

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 kotlin.IndexOutOfBoundsException. #1499

Open
wants to merge 1 commit into
base: jb-main
Choose a base branch
from

Conversation

bennyhuo
Copy link

@bennyhuo bennyhuo commented Aug 15, 2024

Close a ComposeSceneLayer will also remove it from the container, thus making the layers modified while being iterated. Iterating the layers reversely may be the simplest fix since the removal always happens to the last element.

Close a ComposeSceneLayer will also remove it from the container, thus making the layers modified while being iterated. Iterating the layers reversely may be the simplest fix since the removal always happens to the last element.
Copy link
Member

@MatkovIvan MatkovIvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! I would love to see some tests and/or a comment about it, but LGTM as a hot fix.

For the proper fix, I guess we need to split dispose and close in the layer implementation (no need for it in the interface)

@elijah-semyonov
Copy link

Oh, yeah, I've noticed this exact piece of code while investigating a new layers approach. It will be remade soon, but we can merge it until then if it fixes the issue. I'm not actually sure why it doesn't throw the concurrent modification exception in this case.

@bennyhuo
Copy link
Author

Oh, yeah, I've noticed this exact piece of code while investigating a new layers approach. It will be remade soon, but we can merge it until then if it fixes the issue. I'm not actually sure why it doesn't throw the concurrent modification exception in this case.

If you use rememberComposeSceneLayer to maintain a ComposeSceneLayer, it will be closed in a DisposableEffect block. But the code may crash otherwise.

internal fun rememberComposeSceneLayer(
    focusable: Boolean = false
): ComposeSceneLayer {
    ....
    val layer = remember {
        ...
    }
    layer.focusable = focusable
    DisposableEffect(Unit) {
        onDispose {
            layer.close()
        }
    }
    ...
    return layer
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants