Skip to content

Refactoring, thread-safety, and DI improvements in EurekaServerInitializerConfiguration #4570

@alihsan-tsdln

Description

@alihsan-tsdln

Hi team Spring Team,

To open this issue more professionally and cleanly, I used AI to help. Please accept my apologize.

While reviewing the codebase, I came across EurekaServerInitializerConfiguration and noticed a few areas that could benefit from modernization, thread-safety improvements, and consistency.

Before diving into the details, I noticed the inline comment inside the start() method:
// TODO: is this class even needed now?
First and foremost, I wanted to ask if this class is slated for removal or deprecation. If it is going to stay, I would love to submit a PR to clean it up.

Here are the specific points I've identified:

1. Thread Visibility Issue (volatile flag)
The running boolean flag is modified inside a manually created background thread (new Thread(() -> {...}).start()), but it is read by the container via the isRunning() method (likely on a different thread). Currently, it lacks the volatile keyword, which could lead to visibility issues in a concurrent environment.

2. Interface Segregation (Event Publishing)
The class currently autowires the entire ApplicationContext just to publish two events (EurekaRegistryAvailableEvent and EurekaServerStartedEvent). To adhere better to the interface segregation principle, it would be cleaner to inject ApplicationEventPublisher instead of the full context.

3. Dependency Injection Style
The class heavily relies on field injection (@Autowired on fields). Refactoring this to Constructor Injection would make the class immutable, easier to test, and align with current Spring best practices.

4. Inconsistent Injection Mechanisms (Aware Interfaces vs. DI)
The class implements ServletContextAware to obtain the ServletContext, while simultaneously using @Autowired for other framework components like ApplicationContext. In modern Spring, ServletContext can simply be injected via standard dependency injection (preferably Constructor Injection). Removing the Aware interface would standardize the injection mechanism and reduce framework coupling.

5. Raw Thread Creation
Instead of spinning up a raw new Thread(...), it might be safer to utilize a Spring-managed TaskExecutor or ThreadPoolTaskExecutor if asynchronous startup is still strictly required here.

6. Missing Documentation (Javadoc)
The class currently lacks class-level Javadoc explaining its primary role within the Eureka lifecycle, which makes it harder for new contributors to understand its context.

Proposal

If you agree that this class is still needed, I am more than happy to draft a PR to:

  • Add volatile to the running flag.
  • (Optional, based on your feedback) Refactor all field injections and ServletContextAware into a single Constructor Injection block.
  • Replace ApplicationContext with ApplicationEventPublisher.
  • Add descriptive Javadoc to clarify the component's role and lifecycle.
  • (Optional, based on your feedback) Refactor the raw thread creation.

I'm looking forward to contributing to this fix and would love to help out with similar tech-debt/refactoring tasks across the repository in the future.

Let me know your thoughts. Thank You

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions