Fix cloned watcher leaking (#2164)

[lwasyl]

I wanted to add a test case, but couldn't reproduce it without a schema that has a root object, since apollographql/apollo-kotlin-normalized-cache-incubating#78 wouldn't be triggered. I'm aware the names there aren't ideal, but since I'm not sure if there's a more straightforward way to test I didn't invest time into them. The problem with testing I see is that I don't have a reference to the cloned watcher, so the only observable effect is an additional API call. Is there some other way to trigger refetching of open calls? Anyway, suggestions for testing and/or names are welcome.

I didn't add test for all toFlow() variants but applied the same fix. I could try to fit them in a single test case or add a separate test case for each, if needed.

[apollo-cla]

@lwasyl: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[sav007]

I wanted to add a test case, but couldn't reproduce it without a schema that has a root object

Could you please explain what you need, I can make changes to our existing schema.json with whatever you need to test

[lwasyl]

I need something that triggers behavior explained in https://github.com/apollographql/apollo-android/issues/2033, that is:

• query A
• clear cache
• query B <- should trigger refetching for A

so if I read correctly, I need two different queries with intersecting dependentKeys. When I do the same query (so it'd have exactly the same dependentKeys) then refetching is not triggered.

The way I reproduced that is by mimicking something we've been hitting internally, where we have a root object for most queries, so whenever we fetch

root { 
  a
  b
}

clear cache and then fetch a different query

root {
  a
}

then the first query gets refetched. I played with the existing queries and didn't find any pair that triggered this.

[sav007]

hm, what about:

query Query1 {
  hero {
    id
    name
  }
}

query Query2 {
  hero {
    name
  }
}

[lwasyl]

Yep, that works :) I updated the PR

[sav007]

@martinbonnin @tasomaniac

com.apollographql.apollo.gradle.MultiplatformTests > ios framework compiles FAILED
    org.gradle.testkit.runner.UnexpectedBuildFailure: Unexpected build execution failure in /Users/runner/runners/2.169.0/work/apollo-android/apollo-android/apollo-gradle-plugin/build/testProject with arguments [--stacktrace, :linkDebugFrameworkIosArm64]

    Output:
    Daemon will be stopped at the end of the build after running out of JVM memory

    FAILURE: Build failed with an exception.

    * What went wrong:
    Metaspace

    * Try:
    Run with --info or --debug option to get more log output. Run with --scan to get full insights.

    * Exception is:
    java.lang.OutOfMemoryError: Metaspace

We are running out of mem :(

[martinbonnin]

This happens in the gradle plugin tests, right ? I just made it so that gradle is relaunched more often. Hopefully that'll help. In the long run we should investigate with VisualVM see if there's a ClassLoader leak somewhere. I'll create an issue for that.

[tasomaniac]

We recently increased memory to fix CI issues. With Kotlin Multiplatform we use much more memory now. I'm afraid Gradle tests has their own Gradle instance and has a smaller memory. Adding CLI flag or adding gradle.properties file to tests with increased memory should help