spring-cloud / spring-cloud/spring-cloud-netflix

Refactoring, thread-safety, and DI improvements in EurekaServerInitializerConfiguration

Open
#4,570 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

waiting-for-triage
Dominant language
Java
Stars
5k
Forks
2.5k
Avg merge
1d 2h
Merged PRs (30d)
10

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate EurekaServerInitializerConfiguration and begin with the start() method and its TODO comment. First confirm with maintainers whether the class remains needed and which proposed changes are in scope; completion depends on an agreed, testable subset rather than the full list of optional refactors.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.