feat:模块复用基座单例bean时，在模块销毁时不销毁基座bean #207
Conversation
WalkthroughThe pull request introduces several changes to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (13)
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/DefaultApplicationContextFactory.java (1)
25-30: Consider enhancing class documentationThe current documentation could be more descriptive about:
- The class's role in preventing base singleton bean destruction
- Its relationship with BizDefaultListableBeanFactory
- Why it only handles WebApplicationType.NONE
/** - * WebApplicationType.NONE 自定义上下文 + * Custom ApplicationContextFactory for WebApplicationType.NONE that uses BizDefaultListableBeanFactory + * to prevent destruction of base singleton beans when a module is destroyed. + * This factory has the highest precedence and only handles non-web application contexts. * * @author duanzhiqiang * @version DefaultApplicationContextFactory.java, v 0.1 2024年11月12日 17:55 duanzhiqiang */koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationServletEnvironment.java (2)
25-33: Documentation should clarify the purpose and usage contextWhile the documentation mentions this is copied from Spring Boot's internal class, it would be beneficial to explain:
- Why this copy was necessary
- How it fits into the module reuse architecture
- When and how it should be used in the Koupleless context
33-33: Consider documenting the module lifecycle implicationsThis class plays a crucial role in managing the environment for modular applications. Consider:
- Adding detailed documentation about how this class interacts with the module lifecycle
- Creating a diagram showing the relationship between base and module environments
- Adding examples of typical usage patterns
This will help maintainers understand the architectural implications and proper usage patterns.
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationReactiveWebEnvironment.java (2)
25-33: Document the purpose and usage of this classWhile the class includes a reference to the original Spring Boot class, it would be beneficial to document:
- Why this copy was necessary
- How it fits into the module reuse architecture
- The implications of returning null for profile properties
/** * copy from {@link org.springframework.boot.web.reactive.context.ApplicationReactiveWebEnvironment} * due to not public class * {@link StandardReactiveWebEnvironment} for typical use in a typical * {@link SpringApplication}. + * + * This class is part of the module reuse architecture, providing a custom environment + * implementation that prevents profile inheritance between the base module and + * child modules. By returning null for profile properties, it ensures that each module + * maintains its profile configuration independence. * * @author Phillip Webb */
33-49: Consider adding unit tests for this environment classSince this class plays a crucial role in module isolation, it would be beneficial to add unit tests verifying:
- Profile property methods return null as expected
- Property resolver creation works correctly
- Integration with the module reuse system
Would you like me to help create a comprehensive test suite for this class?
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizAnnotationConfigReactiveWebServerApplicationContext.java (1)
45-49: Improve Javadoc comment for clarity and consistencyThe Javadoc comment mixes Chinese and English, which may affect readability and understanding. Consider revising the comment for better clarity and consistency.
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizAnnotationConfigServletWebServerApplicationContext.java (2)
46-49: Unnecessary Overrides of 'getBean' MethodsThe
getBeanmethods are overridden but simply delegate to the superclass without adding any additional functionality. Unless there is a specific need for these overrides, consider removing them to reduce redundancy and simplify the code.Apply this diff to remove the unnecessary overrides:
- @Override - public Object getBean(String name) throws BeansException { - return super.getBean(name); - } - - @Override - public <T> T getBean(Class<T> requiredType, Object... args) throws BeansException { - return super.getBean(requiredType, args); - }Also applies to: 51-54
30-34: Consider Writing Code Comments in EnglishTo enhance code maintainability and facilitate collaboration in an international development environment, consider writing code comments in English.
Also applies to: 38-41, 57-60, 81-84
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizDefaultListableBeanFactory.java (5)
28-32: Class documentation languageThe class documentation and comments are in Chinese. If the codebase is intended for an international audience or follows English as the standard for code comments, consider translating these comments to English to improve accessibility and maintain consistency.
Example translation:
/** * Overrides the destroy methods to prevent destruction of base beans when they are reused in Biz modules. * * @author ... * @version ... */
19-23: Unused importsThe import
org.springframework.lang.Nullable;is not used in the code provided. Unused imports can clutter the code and should be removed.Apply this diff to remove the unused import:
-import org.springframework.lang.Nullable;
100-105: Missing@Overrideannotation ondestroyBeanmethodThe
destroyBeanmethod overrides a method from the superclass but lacks the@Overrideannotation. Adding the annotation improves code readability and helps catch potential issues if the superclass method signature changes.Add the
@Overrideannotation:@Override protected void destroyBean(String beanName, @Nullable DisposableBean bean) { // method implementation }Ensure that you import the appropriate
Nullableannotation if it's required.
44-47: Constructor can call superclass default constructor implicitlyThe explicit call to
super();in the constructor is unnecessary since it would be called implicitly if no other superclass constructors are called.You can remove
super();to simplify the code:-public BizDefaultListableBeanFactory() { - super(); +public BizDefaultListableBeanFactory() { this.isBaseBeanFactory = !isOnBiz(); }
52-53: Use descriptive names for thread-local variablesThe thread-local variable
CUR_DESTROY_SINGLE_BEAN_HOLDERcould be renamed for clarity and to follow naming conventions. Consider using camelCase for variable names and making it clear that it's a ThreadLocal variable.Rename the variable:
-private static final ThreadLocal<Object> CUR_DESTROY_SINGLE_BEAN_HOLDER = new ThreadLocal<>(); +private static final ThreadLocal<Object> curDestroySingletonBeanHolder = new ThreadLocal<>();Ensure that you update all usages of this variable accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
koupleless-base-plugin/pom.xml(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationReactiveWebEnvironment.java(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationServletEnvironment.java(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizAnnotationConfigReactiveWebServerApplicationContext.java(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizAnnotationConfigServletWebServerApplicationContext.java(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizDefaultListableBeanFactory.java(1 hunks)koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/DefaultApplicationContextFactory.java(1 hunks)koupleless-base-plugin/src/main/resources/META-INF/spring.factories(1 hunks)
🔇 Additional comments (13)
koupleless-base-plugin/src/main/resources/META-INF/spring.factories (2)
15-17: LGTM: Improved readability with proper spacing
The added newline improves the readability of the configuration file by clearly separating different sections.
18-21: Verify the implementation of custom ApplicationContextFactory classes
The addition of custom ApplicationContextFactory implementations aligns with the PR objective of customizing bean destruction workflow. However, we should verify that these implementations properly handle the singleton bean lifecycle.
Consider documenting the following aspects in the codebase:
- The lifecycle management strategy for base singleton beans
- The interaction between module contexts and the base context
- The conditions under which beans are preserved vs destroyed
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/DefaultApplicationContextFactory.java (3)
1-18: LGTM: License and package declaration are correct
19-24: LGTM: Imports are appropriate and minimal
31-42: Implementation looks good, but verify BizDefaultListableBeanFactory
The implementation correctly uses BizDefaultListableBeanFactory to handle bean lifecycle management, which aligns with the PR objective of preventing base singleton bean destruction.
Let's verify the BizDefaultListableBeanFactory implementation:
✅ Verification successful
BizDefaultListableBeanFactory correctly prevents base singleton bean destruction
The implementation properly overrides bean destruction methods to ensure that base singleton beans are not destroyed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BizDefaultListableBeanFactory implementation
echo "Searching for BizDefaultListableBeanFactory implementation..."
ast-grep --pattern 'class BizDefaultListableBeanFactory extends DefaultListableBeanFactory {
$$$
}'
# Look for bean destruction related overrides
echo "Checking for bean destruction related methods..."
rg -A 10 "destroySingleton|destroyBean" --type java
Length of output: 6831
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationServletEnvironment.java (2)
45-48: LGTM! Property resolver implementation follows Spring Boot's approach
The implementation correctly uses ConfigurationPropertySources to create the property resolver, which is consistent with Spring Boot's standard approach for configuration property handling.
35-43: Verify the impact of null profile properties
The implementation returns null for both active and default profiles, which means:
- No predefined profiles will be loaded
- Profile resolution will fall back to parent context behavior
This appears intentional for module reuse, but please verify this doesn't cause issues with profile-specific configurations in the modules.
✅ Verification successful
Further investigate profile usage in MavenUtils.java
Please run the following script to extract profile-related method usages in the specified file:
Further investigate profile-related usages in MavenUtils.java
Please run the following script to comprehensively search for profile-related usages in the specified file:
Verified null profile properties do not impact profile configurations
Returning null for both active and default profiles ensures that profile management is handled externally via mavenSession, preventing any conflicts with module contexts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for profile-specific configuration files or profile usage in the codebase
echo "Searching for profile-specific configuration files..."
fd -e yml -e yaml -e properties | xargs rg -l "^spring\.profiles"
echo "Searching for profile-related code..."
rg -l "setActiveProfiles|setDefaultProfiles|getActiveProfiles|getDefaultProfiles"
Length of output: 502
Script:
#!/bin/bash
# Extract profile-related method usages in MavenUtils.java
ast-grep --pattern $'class MavenUtils {
$$$
setActiveProfiles($_)
$$$
}' koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/utils/MavenUtils.java
Length of output: 228
Script:
#!/bin/bash
# Comprehensive search for profile-related usages in MavenUtils.java
rg "spring\.profiles|setActiveProfiles|setDefaultProfiles|getActiveProfiles|getDefaultProfiles" \
koupleless-ext/koupleless-build-plugin/koupleless-base-build-plugin/src/main/java/com/alipay/sofa/koupleless/base/build/plugin/utils/MavenUtils.java
Length of output: 323
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/ApplicationReactiveWebEnvironment.java (3)
1-16: LGTM: Proper license header included
The file includes the appropriate Apache Software Foundation (ASF) license header.
46-48: LGTM: Proper delegation to Spring Boot's configuration property resolver
The implementation correctly uses Spring Boot's ConfigurationPropertySources to create the property resolver, maintaining consistency with Spring Boot's property resolution mechanism.
36-43: Verify the impact of returning null for profile properties
Both doGetActiveProfilesProperty() and doGetDefaultProfilesProperty() return null, which differs from the standard Spring behavior. This is likely intentional to prevent profile inheritance, but we should verify this doesn't cause issues in edge cases.
✅ Verification successful
Verified: Returning null for profile properties does not introduce issues based on current usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for profile-related configurations and usages
echo "Searching for profile configurations in the codebase..."
rg -l "spring\.profiles\."
# Search for potential profile property references
echo "Searching for profile property references..."
ast-grep --pattern 'getActiveProfiles()'
ast-grep --pattern 'getDefaultProfiles()'
Length of output: 453
koupleless-base-plugin/pom.xml (1)
66-70: LGTM! Dependency addition looks good.
The spring-web dependency with provided scope is appropriately added to support the new web environment classes while preventing transitive inclusion.
Let's verify the Spring version management:
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizDefaultListableBeanFactory.java (2)
117-128: Ensure proper cleanup in destroySingleton
In the destroySingleton method, when !isBaseBeanFactory is true, you set a thread-local variable CUR_DESTROY_SINGLE_BEAN_HOLDER but only remove it in the finally block. Ensure that all possible exceptions are correctly handled and that CUR_DESTROY_SINGLE_BEAN_HOLDER.remove(); is always executed to prevent memory leaks.
Confirm that the superclass destroySingleton method cannot throw unchecked exceptions that could bypass the finally block. If necessary, wrap the super.destroySingleton(beanName); call within its own try-catch block to ensure CUR_DESTROY_SINGLE_BEAN_HOLDER.remove(); is always called.
136-142: Thread safety and concurrency concerns with BASE_RESUE_BEAN_SET
While BASE_RESUE_BEAN_SET is a concurrent set, ensure that all modifications and reads are thread-safe and that the use of ThreadLocal variables aligns with the intended concurrency model.
Review the usage of BASE_RESUE_BEAN_SET and CUR_DESTROY_SINGLE_BEAN_HOLDER to confirm that they function correctly in a multithreaded environment. Pay special attention to potential race conditions or memory consistency errors.
| return (webApplicationType != WebApplicationType.REACTIVE) ? null | ||
| : ApplicationReactiveWebEnvironment.class; |
There was a problem hiding this comment.
Undefined class ApplicationReactiveWebEnvironment
The class ApplicationReactiveWebEnvironment is referenced in methods getEnvironmentType and createEnvironment but is not imported or defined in the provided code. This will lead to compilation errors.
Ensure that ApplicationReactiveWebEnvironment is properly defined and imported.
Also applies to: 60-61
| */ | ||
| private ConfigurableApplicationContext createContext() { | ||
| //自定义BeanFactory的销毁 | ||
| DefaultListableBeanFactory beanFactory = new BizDefaultListableBeanFactory(); |
There was a problem hiding this comment.
Undefined class BizDefaultListableBeanFactory
At line 76, BizDefaultListableBeanFactory is instantiated but not imported or defined in the current context, which may cause compilation issues.
Make sure that BizDefaultListableBeanFactory is defined and imported correctly.
| return (webApplicationType != WebApplicationType.SERVLET) ? null | ||
| : ApplicationServletEnvironment.class; | ||
| } |
There was a problem hiding this comment.
Ensure 'ApplicationServletEnvironment' is Defined and Imported
The class ApplicationServletEnvironment is used but not imported. If this is a custom class, please ensure it is properly defined and imported to avoid compilation errors.
Apply this diff to add the necessary import statement:
+import org.springframework.boot.context.ApplicationServletEnvironment;Also applies to: 71-73
| */ | ||
| private ConfigurableApplicationContext createContext() { | ||
| //自定义BeanFactory的销毁 | ||
| DefaultListableBeanFactory beanFactory = new BizDefaultListableBeanFactory(); |
There was a problem hiding this comment.
Ensure 'BizDefaultListableBeanFactory' is Defined and Imported
The class BizDefaultListableBeanFactory is used but not imported. If this is a custom class, please ensure it is properly defined and imported to avoid compilation errors.
Apply this diff to add the necessary import statement:
+import com.alipay.sofa.koupleless.plugin.context.BizDefaultListableBeanFactory;Committable suggestion skipped: line range outside the PR's diff.
| if (isSingleton(name) && isOnBiz() && isBaseBeanFactory) { | ||
| BASE_RESUE_BEAN_SET.add(bean); |
There was a problem hiding this comment.
Logical inconsistency in condition
The condition if (isSingleton(name) && isOnBiz() && isBaseBeanFactory) may never be true due to a logical inconsistency. Since isBaseBeanFactory is initialized as !isOnBiz() in the constructor (line 46), isOnBiz() and isBaseBeanFactory cannot both be true simultaneously. This means the condition will always evaluate to false, and the block within will never execute.
Please review and correct the logic of this condition. If the intent is to record reused base beans when on Biz and the bean factory is not the base, the condition should be:
if (isSingleton(name) && isOnBiz() && !isBaseBeanFactory)| } catch (Exception e) { | ||
| logger.error("记录复用基座bean异常", e); | ||
| } |
There was a problem hiding this comment.
Undefined logger variable
The logger used in the catch block is not defined within the class. This will lead to a compilation error. You need to define a logger instance, typically using a logging framework like SLF4J or Log4j.
Add the logger definition at the class level:
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class BizDefaultListableBeanFactory extends DefaultListableBeanFactory {
private static final Logger logger = LoggerFactory.getLogger(BizDefaultListableBeanFactory.class);
//... rest of the codeEnsure that you have the appropriate logging library included in your project dependencies.
| T bean = super.doGetBean(name, requiredType, args, typeCheckOnly); | ||
| try { | ||
| //复用基座的bean时 记录下复用的基座bean | ||
| if (isSingleton(name) && isOnBiz() && isBaseBeanFactory) { | ||
| BASE_RESUE_BEAN_SET.add(bean); | ||
| } | ||
| } catch (Exception e) { | ||
| logger.error("记录复用基座bean异常", e); | ||
| } | ||
| return bean; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Exception handling best practices
Catching the generic Exception is not recommended as it may catch unintended exceptions and hide underlying issues. It's better to catch specific exceptions or allow them to propagate if they are unexpected.
Modify the catch block to catch specific exceptions. For example, if you expect a RuntimeException or any specific exception, catch that instead:
try {
//...
} catch (SpecificException e) {
logger.error("An error occurred while recording reused base bean", e);
}If there's no specific exception to catch, consider whether the try-catch block is necessary or if the exception should be allowed to propagate.
| if (!isBaseBeanFactory && isBaseBean()) { | ||
| //传为null时不进行销毁 | ||
| super.destroyBean(beanName, null); | ||
| return; | ||
| } | ||
| super.destroyBean(beanName, bean); |
There was a problem hiding this comment.
Potential issue with passing null to destroyBean
In destroyBean, when the condition if (!isBaseBeanFactory && isBaseBean()) is true, you call super.destroyBean(beanName, null); passing null for the bean parameter. If the superclass method does not handle a null bean appropriately, this could lead to a NullPointerException.
Review the superclass implementation to ensure that passing null is safe. If not, consider retrieving the bean instance before calling destroyBean:
Object beanInstance = getSingleton(beanName);
super.destroyBean(beanName, beanInstance);Or adjust the logic to avoid passing null.
| ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); | ||
| return contextClassLoader == null | ||
| || BIZ_CLASSLOADER.equals(contextClassLoader.getClass().getName()); | ||
| } |
There was a problem hiding this comment.
Potential logical error in isOnBiz() method
The isOnBiz() method returns true when contextClassLoader is null. This might not be the intended behavior, as a null class loader could indicate an unexpected state. Typically, if the class loader is null, it's safer to assume that it's not on Biz.
Consider revising the condition:
return contextClassLoader != null
&& BIZ_CLASSLOADER.equals(contextClassLoader.getClass().getName());This change ensures that isOnBiz() returns true only when the contextClassLoader is not null and matches the expected Biz class loader.
| private static final Set<Object> BASE_RESUE_BEAN_SET = Collections | ||
| .newSetFromMap(new ConcurrentHashMap<>()); |
There was a problem hiding this comment.
Typo in variable name BASE_RESUE_BEAN_SET
The variable name BASE_RESUE_BEAN_SET appears to be a typo. It should likely be BASE_REUSE_BEAN_SET to accurately reflect its purpose of storing reused base beans.
Apply this diff to correct the typo:
-private static final Set<Object> BASE_RESUE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>());
+private static final Set<Object> BASE_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static final Set<Object> BASE_RESUE_BEAN_SET = Collections | |
| .newSetFromMap(new ConcurrentHashMap<>()); | |
| private static final Set<Object> BASE_REUSE_BEAN_SET = Collections | |
| .newSetFromMap(new ConcurrentHashMap<>()); |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizDefaultListableBeanFactory.java (2)
27-34: Consider adding English comments for broader maintainabilityThe class-level comments are currently in Chinese. To enhance maintainability and ensure that a wider range of developers can understand and contribute to the codebase, consider providing comments in English or adding English translations.
120-124: Improve reliability ofisOnBiz()methodThe
isOnBiz()method relies on checking the class loader's class name as a string, which may be fragile if class names change or are obfuscated.Consider using an
instanceofcheck if theBizClassLoaderclass is accessible:private boolean isOnBiz() { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); return contextClassLoader instanceof com.alipay.sofa.ark.container.service.classloader.BizClassLoader; }If the class is not accessible, ensure that the string comparison is robust and consider handling possible variations in the class loader's name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
koupleless-base-plugin/src/main/java/com/alipay/sofa/koupleless/plugin/context/BizDefaultListableBeanFactory.java(1 hunks)
| /** | ||
| * 基座bean复用bean集合引用 | ||
| */ | ||
| private static final Set<Object> BASE_FACTORTY_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>()); |
There was a problem hiding this comment.
Typo in variable name BASE_FACTORTY_REUSE_BEAN_SET
The variable name BASE_FACTORTY_REUSE_BEAN_SET appears to have a typo. It should likely be BASE_FACTORY_REUSE_BEAN_SET to accurately represent its purpose.
Apply this diff to correct the typo:
-private static final Set<Object> BASE_FACTORTY_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>());
+private static final Set<Object> BASE_FACTORY_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>());Ensure that all references to this variable are updated accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static final Set<Object> BASE_FACTORTY_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>()); | |
| private static final Set<Object> BASE_FACTORY_REUSE_BEAN_SET = Collections.newSetFromMap(new ConcurrentHashMap<>()); |
| protected <T> T doGetBean(String name, Class<T> requiredType, Object[] args, | ||
| boolean typeCheckOnly) throws BeansException { | ||
|
|
||
| T bean = super.doGetBean(name, requiredType, args, typeCheckOnly); | ||
|
|
||
| // 只有是基座isBaseBeanFactory 但获取bean时是模块发起调用（即复用基座的bean时） 记录下复用的基座bean | ||
| if (isBaseBeanFactory && isOnBiz() && isSingleton(name)) { | ||
| BASE_FACTORTY_REUSE_BEAN_SET.add(bean); | ||
| } | ||
|
|
||
| return bean; | ||
| } |
There was a problem hiding this comment.
Potential memory leak due to static BASE_FACTORY_REUSE_BEAN_SET holding bean references
The static Set BASE_FACTORY_REUSE_BEAN_SET stores references to beans added in the doGetBean method. Since these beans are held in a static collection and are never removed, they may prevent the beans from being garbage collected, leading to a memory leak over time.
Consider implementing a mechanism to remove bean references from BASE_FACTORY_REUSE_BEAN_SET when they are no longer needed. For example, you could remove the bean from the set when it is no longer in use or when the context is destroyed.
Apply this diff to illustrate the addition of a removal mechanism:
// In your `requiresDestruction` method or appropriate lifecycle method
if (!isBaseBeanFactory && isBaseReuseBean(bean)) {
// Do not register DisposableBean
return false;
+ // Remove the bean from the reuse set to prevent memory leaks
+ BASE_FACTORY_REUSE_BEAN_SET.remove(bean);
}Ensure that this removal aligns with the desired bean lifecycle management.
Committable suggestion skipped: line range outside the PR's diff.
1、基于Spring boot启动预留的ApplicationContextFactory，自定义DefaultListableBeanFactory来扩展
2、当模块获取基座单例时bean 记录引用（由于无法通过名称获取bean 因为在子模块中 beanName是可以重新定义的）
3、在模块创建这个复用的bean时 不注册销毁行为 支持复用bean为在子模块中注册单例或者是其他scope
4、通过记录的复用的bean 引用来判断是基座复用bean
Summary by CodeRabbit
New Features
Improvements
Documentation
spring.factoriesto include new context factory implementations.