Skip to content

Revert "[fix][ci] Fix tests memory leak due to mockito-inline (#15513)" - #16820

Closed
nodece wants to merge 1 commit into
apache:masterfrom
nodece:revert-15513
Closed

Revert "[fix][ci] Fix tests memory leak due to mockito-inline (#15513)"#16820
nodece wants to merge 1 commit into
apache:masterfrom
nodece:revert-15513

Conversation

@nodece

@nodece nodece commented Jul 27, 2022

Copy link
Copy Markdown
Member

This reverts commit 14b95ec.

Fixes #16582

Motivation

#15513 uses clearInlineMocks() method to fix the memory leak, but it looks to introduce a new issue:

org.mockito.exceptions.base.MockitoException: Unable to create mock instance of type 'ServerCnx'
  	at org.apache.pulsar.broker.BrokerTestUtil.spyWithClassAndConstructorArgs(BrokerTestUtil.java:43)
  	at org.apache.pulsar.broker.service.PersistentTopicTest.setup(PersistentTopicTest.java:226)
  	at org.apache.pulsar.broker.service.persistent.PersistentTopicStreamingDispatcherTest.setup(PersistentTopicStreamingDispatcherTest.java:34)
  	at jdk.internal.reflect.GeneratedMethodAccessor599.invoke(Unknown Source)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
  	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
  	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
  	at org.testng.internal.MethodInvocationHelper.invokeMethodConsideringTimeout(MethodInvocationHelper.java:61)
  	at org.testng.internal.ConfigInvoker.invokeConfigurationMethod(ConfigInvoker.java:366)
  	at org.testng.internal.ConfigInvoker.invokeConfigurations(ConfigInvoker.java:320)
  	at org.testng.internal.TestInvoker.runConfigMethods(TestInvoker.java:701)
  	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:527)
  	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
  	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
  	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
  	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
  	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
  	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
  	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
  	at org.testng.TestRunner.privateRun(TestRunner.java:764)
  	at org.testng.TestRunner.run(TestRunner.java:585)
  	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
  	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
  	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
  	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
  	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
  	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
  	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
  	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
  	at org.testng.TestNG.runSuites(TestNG.java:1069)
  	at org.testng.TestNG.run(TestNG.java:1037)
  	at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:135)
  	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:112)
  	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeLazy(TestNGDirectoryTestSuite.java:123)
  	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:90)
  	at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:146)
  	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
  	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
  	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
  	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)
  Caused by: org.mockito.creation.instance.InstantiationException: 
  Unable to create instance of 'ServerCnx'.
  Please ensure the target class has a constructor that matches these argument types: [org.apache.pulsar.broker.PulsarService] and executes cleanly.
  	... 40 more
  Caused by: java.lang.reflect.InvocationTargetException
  	at org.mockito.internal.util.reflection.InstrumentationMemberAccessor.newInstance(InstrumentationMemberAccessor.java:198)
  	at org.mockito.internal.util.reflection.InstrumentationMemberAccessor.newInstance(InstrumentationMemberAccessor.java:161)
  	at org.mockito.internal.util.reflection.ModuleMemberAccessor.newInstance(ModuleMemberAccessor.java:42)
  	at org.mockito.internal.creation.instance.ConstructorInstantiator.invokeConstructor(ConstructorInstantiator.java:70)
  	at org.mockito.internal.creation.instance.ConstructorInstantiator.withParams(ConstructorInstantiator.java:53)
  	at org.mockito.internal.creation.instance.ConstructorInstantiator.newInstance(ConstructorInstantiator.java:39)
  	at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.doCreateMock(InlineDelegateByteBuddyMockMaker.java:360)
  	at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.createMock(InlineDelegateByteBuddyMockMaker.java:330)
  	at org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.createMock(InlineByteBuddyMockMaker.java:58)
  	at org.mockito.internal.util.MockUtil.createMock(MockUtil.java:53)
  	at org.mockito.internal.MockitoCore.mock(MockitoCore.java:84)
  	at org.mockito.Mockito.mock(Mockito.java:1964)
  	... 40 more
  Caused by: java.lang.ClassCastException: class org.apache.pulsar.broker.service.BrokerService cannot be cast to class org.apache.pulsar.broker.resources.PulsarResources (org.apache.pulsar.broker.service.BrokerService and org.apache.pulsar.broker.resources.PulsarResources are in unnamed module of loader 'app')
  	at org.apache.pulsar.broker.PulsarService.getPulsarResources(PulsarService.java:265)
  	at org.apache.pulsar.broker.service.TopicListService.<init>(TopicListService.java:103)
  	at org.apache.pulsar.broker.service.ServerCnx.<init>(ServerCnx.java:279)
  	at org.apache.pulsar.broker.service.ServerCnx.<init>(ServerCnx.java:239)
  	at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:732)
  	at org.mockito.internal.util.reflection.InstrumentationMemberAccessor$Dispatcher$ByteBuddy$kV10z9EX.invokeWithArguments(Unknown Source)
  	at org.mockito.internal.util.reflection.InstrumentationMemberAccessor.lambda$newInstance$0(InstrumentationMemberAccessor.java:191)
  	at org.mockito.internal.util.reflection.InstrumentationMemberAccessor.newInstance(InstrumentationMemberAccessor.java:188)
  	... 51 more

I see a description of clearInlineMocks() that seems to be relevant to this problem, so try to revert #15513.

In certain specific, rare scenarios (issue #1619) inline mocking causes memory leaks. There is no clean way to mitigate this problem completely. Hence, we introduced a new API to explicitly clear mock state (only make sense in inline mocking!). See example usage in MockitoFramework.clearInlineMocks(). If you have feedback or a better idea how to solve the problem please reach out.

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@nodece
nodece requested review from nicoloboschi and removed request for nicoloboschi July 27, 2022 11:01

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't see any suggestion in the javadoc that indicates the usage of clearInlineMocks() could cause the error we're facing, would you mind to indicate more details? @nodece

@nodece

nodece commented Jul 27, 2022

Copy link
Copy Markdown
Member Author

I don't see any suggestion in the javadoc that indicates the usage of clearInlineMocks() could cause the error we're facing, would you mind to indicate more details? @nodece

I just guess the log is related to #15513.

In certain specific, rare scenarios (issue mockito/mockito#1619) inline mocking causes memory leaks. There is no clean way to mitigate this problem completely. Hence, we introduced a new API to explicitly clear mock state (only make sense in inline mocking!). See example usage in MockitoFramework.clearInlineMocks(). If you have feedback or a better idea how to solve the problem please reach out.

Link to https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#47

@shibd

shibd commented Jul 27, 2022

Copy link
Copy Markdown
Member

Hi, @nodece. About ClassCastException , in #16821 will fixed. I tested the code after the revert and an exception still will appear.

@nodece

nodece commented Jul 28, 2022

Copy link
Copy Markdown
Member Author

@shibd Thank you for your testing, I close this PR.

@nodece nodece closed this Jul 28, 2022
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.

Test: mockito broken

3 participants