Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Mar 4, 2022

concat produces a lazy sequence, which makes its repeated usage in a
loop a potential stack overflow hazard (when the produced seq gets
realized and has to recur over its subseqs).

Background: I've been trying to introduce clojure to a java/kotlin project. Clojure tests kept on dying with a StackOverflowError in CI during test discovery, with stacktrace mentioning concat. I couldn't debug the issue very well - it wasn't reliably reproducible locally - but noticed that apparently all AOTted classes in test classpath were being scanned for tests, which is a lot of classes. I've run grep on jovial, found a single use of concat in a loop and tried to replace it with a non-lazy equivalent. Tests run fine with the patched version. I'm not 100% convinced that concat's laziness was the sole culprit due to flakiness of local test runs, but if it helps it helps.

`concat` produces a lazy sequence, which makes its repeated usage in a
loop a potential stack overflow hazard (when the produced seq gets
realized and has to recur over its subseqs).
@ghost
Copy link
Author

ghost commented Mar 7, 2022

I've actually taken the time to sit down and repro the overflow, it's proven to not be too hard. It's here, but I suspect tweaking the numbers in infinite-fns and/or creating more namespaces (and importing them in the test) might be necessary depending on the local JVM, machine and alignment of stars.

The stacktrace in JUnit report (truncated, of course):

org.gradle.api.internal.tasks.testing.TestSuiteExecutionException: Could not complete execution for Gradle Test Executor 1.
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:63)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at [email protected]/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at [email protected]/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at [email protected]/java.lang.reflect.Method.invoke(Method.java:568)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy1/jdk.proxy1.$Proxy2.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: org.junit.platform.commons.JUnitException: TestEngine with ID 'org.ajoberstar.jovial.clojure-test' failed to discover tests
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discoverEngineRoot(EngineDiscoveryOrchestrator.java:111)
	at org.junit.platform.launcher.core.EngineDiscoveryOrchestrator.discover(EngineDiscoveryOrchestrator.java:85)
	at org.junit.platform.launcher.core.DefaultLauncher.discover(DefaultLauncher.java:92)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
	... 18 more
Caused by: java.lang.StackOverflowError
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:51)
	at clojure.lang.RT.seq(RT.java:535)
	at clojure.core$seq__5419.invokeStatic(core.clj:139)
	at clojure.core$concat$fn__5510.invoke(core.clj:727)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:51)
	at clojure.lang.RT.seq(RT.java:535)
	at clojure.core$seq__5419.invokeStatic(core.clj:139)
	at clojure.core$concat$fn__5510.invoke(core.clj:727)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:51)
	at clojure.lang.RT.seq(RT.java:535)

@ajoberstar ajoberstar merged commit 32a98bf into clojurephant:main Aug 14, 2022
@ajoberstar
Copy link
Member

Apologies for the delay on this, I lost track of this PR being open. I appreciate the work you did to dig into this and fix this! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

1 participant