Giter VIP home page Giter VIP logo

Comments (7)

DavideCannizzo avatar DavideCannizzo commented on May 23, 2024 1

Hey, @Zhou-Kt, thanks for your appreciation and for closing this issue from here. While I don't think such problems can be universally fixed from Kotlin's side, this specific one could indeed be worked around by changing the way singletons are generated.

So I created this YouTrack issue. If you wish, you can track any progress there.

from compose-multiplatform.

DavideCannizzo avatar DavideCannizzo commented on May 23, 2024

First, I created a Minimal Reproducible Example (MRE) that presents the same exact issue:

abstract class Route {
    object Home : Route()

    object Personal : Route()

    companion object {
        @JvmStatic val home = Home
        @JvmStatic val personal = Personal
    }
}

fun main() {
    Route.Home // this causes the issue
    println(Route.home) // null
    println(Route.personal) // Route$Personal@somehash
}

In the MRE,

  1. I got rid of Compose entirely — in fact I wouldn't expect the Compose compiler to do anything about those classes given they don't even contain @Composables or anything.
  2. I replaced the sealed modifier of Route with abstract — in fact sealed does nothing besides providing additional guarantees from the Kotlin compiler about which subclasses are allowed, and similar guarantees by the JVM because of a JVM attribute added to it.
  3. I removed the data modifier from Home and Personal — in fact data does nothing besides auto-generating implementations for Any.hashCode(), Any.equals(Any?), and Any.toString(), plus generating a copy method and componentN() methods for constructor properties (and none of these members are referenced in the MRE, so they're irrelevant).
  4. I removed the Info class entirely, as I felt that that was unrelated and that the real culprit was the Route.Home reference in main.
  5. I replaced the Route.list companion field with a home and a personal fields initialized to the singleton instances of Home and Personal, respectively — this modification maintains the same side effects of the instance constructor of Route.Companion and therefore of its static constructor as well (that is, excluding that listOf was the issue, which seemed unrelated).
  6. I added the @JvmField annotation to the Route.home and Route.personal fields to get us rid of redundant getters for them and to make them static so we can forget of the generated Route.Companion class.

Interestingly, when removing the first line inside the main function, where Route.Home is first referenced, the issue disappears.

Suspecting that the issue would be about the side effects of the static constructors initializing fields, I wanted to know the precise order in which they were called. Therefore, I set out to insert some println statements to debug. However, there's no way to add a static constructor to an object definition in Kotlin code, because init inside an object definition is its instance constructor. For this purpose, I converted the object definitions to plain classes and defined the static field INSTANCE (see [1]) from within Kotlin code. The result is in this Kotlin playground.

Note that in this modified example, the Route.Home reference inside main is no longer referencing to the singleton instance of Route.Home as it did when using Kotlin objects, because that would now be Route.Home.INSTANCE. It is instead just forcing the JVM to load the Route.Home class, which in turn calls its static constructor, which still initializes the INSTANCE field. Therefore, the difference is just that we got rid of a field read, but field reads have no side effects in a single-threaded environment. Plus, interestingly, if we load Route before we load Home (as shown in the first line inside main), the issue disappears. Which leads us to believe that the the issue is a side effect of loading Home before Route.

By running the same Kotlin playground, we can observe this output:

Attempting to call Home.<clinit>
Route.<clinit> start
Route.<clinit> after Route.home is initialized
Personal.<clinit> start
Personal.<clinit> end
Route.<clinit> end
Home.<clinit> start
Home.<clinit> end
Attempting to call Route.<clinit>
null
Route$Personal@7530d0a

This tells us that Route.home is initialized (and therefore Home.INSTANCE referenced) before the static constructor of Home is called, which is where Home.INSTANCE would be initialized. Which means that when Route.home is initialized, Home.INSTANCE is still uninitialized, and therefore null as the JVM specifies.

By inspecting the generated bytecode, it seems like the Kotlin compiler didn't make any mistakes, so I would conclude that this is actually a JVM issue, whether it's intended behavior or not. Whether the Kotlin compiler could spot situations like this and introduce a fix is another matter.

The fact is, when Home is loaded, as explained above, a new instance of it is created. However, Home inherits from Route, which means Route must be loaded before the instance constructor of Home can be called. And loading Route has the side effect of calling its own static constructor. Which explains why the Route's static constructor is invariantly invoked before Home's, which makes it so Route.home is initialized before Home.INSTANCE is initialized. And by the time Route.personal is getting initialized, Route.home was already initialized, and so Route was as well.

This looks like a cyclic reference in JVM class loading.

A quick fix would be to lazy-initialize Route.list, as follows:

sealed class Route {
    data object Home : Route()

    data object Personal: Route()

    companion object {
        val list by lazy { listOf(Home, Personal) }
    }
}

To further exclude Kotlin from causing this issue, here's a Java equivalent that results in the same:

public class Program {

    public static abstract class Route {
    
        public static final Home home = Home.INSTANCE;
        public static final Personal personal = Personal.INSTANCE;
    
        public static class Home extends Route {
            private Home() {}
            public static Home INSTANCE = new Home();
        }

        public static class Personal extends Route {
            private Personal() {}
            public static Personal INSTANCE = new Personal();
        }
    }

    public static void main(String[] args) {
        var x = Route.Home.INSTANCE;
        System.out.println(Route.home);
        System.out.println(Route.personal);
    }
}

*[1]: A static INSTANCE field is generated by the Kotlin compiler in the class of each object definition to store its singleton instance.

from compose-multiplatform.

Zhou-Kt avatar Zhou-Kt commented on May 23, 2024

Wow, that's so professional👍. Thank you for your help.

from compose-multiplatform.

Zhou-Kt avatar Zhou-Kt commented on May 23, 2024

@DavideCannizzo my bad, i've reopened the issue and switched to your title (the old title isn't appropriate).

from compose-multiplatform.

DavideCannizzo avatar DavideCannizzo commented on May 23, 2024

@Zhou-Kt, I'm not sure if it is of any use to keep this issue open — after all, this doesn't depend on Compose, and at best could be worked around in Kotlin compiler, where I opened a separate issue via YouTrack which I linked in my previous comment.

Anyway, at this point we can wait and see what the Compose Multiplatform team says. I'm just a user of this library, and I don't even have the permission to close this issue.

from compose-multiplatform.

Zhou-Kt avatar Zhou-Kt commented on May 23, 2024

@Zhou-Kt, I'm not sure if it is of any use to keep this issue open — after all, this doesn't depend on Compose, and at best could be worked around in Kotlin compiler, where I opened a separate issue via YouTrack which I linked in my previous comment.

Anyway, at this point we can wait and see what the Compose Multiplatform team says. I'm just a user of this library, and I don't even have the permission to close this issue.

Yes, at first I thought it was the effect of mutableStateListOf, so that I brought the issue here. One more issue means one more chance to be solved.

from compose-multiplatform.

m-sasha avatar m-sasha commented on May 23, 2024

Thanks, @DavideCannizzo, for the detailed analysis.

As there's nothing for the Compose Multiplatform team to do here, I'm closing the ticket.

from compose-multiplatform.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.