Do not expose unstarted service, stop started service leak by basilevs · Pull Request #938 · eclipse-equinox/p2

basilevs · 2025-08-30T14:33:46Z

There is a potential for client to see service before they are started.
Dynamically registered services may be never stopped causing resource leaks.

This is a part of #939 in context of #937.

eclipse-equinox-bot · 2025-08-30T14:37:52Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

features/org.eclipse.equinox.p2.core.feature/feature.xml
features/org.eclipse.equinox.p2.extras.feature/feature.xml
features/org.eclipse.equinox.p2.rcp.feature/feature.xml
features/org.eclipse.equinox.p2.sdk/feature.xml
features/org.eclipse.equinox.p2.user.ui/feature.xml
features/org.eclipse.equinox.server.p2/feature.xml

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From a6ebb62ee4a79d3cc4b8fa7ab26ce86eb4ba1661 Mon Sep 17 00:00:00 2001
From: Eclipse Equinox Bot <equinox-bot@eclipse.org>
Date: Mon, 1 Sep 2025 13:45:39 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/features/org.eclipse.equinox.p2.core.feature/feature.xml b/features/org.eclipse.equinox.p2.core.feature/feature.xml
index 4fd7e5946..b0b616d46 100644
--- a/features/org.eclipse.equinox.p2.core.feature/feature.xml
+++ b/features/org.eclipse.equinox.p2.core.feature/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.core.feature"
       label="%featureName"
-      version="1.7.800.qualifier"
+      version="1.7.900.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
diff --git a/features/org.eclipse.equinox.p2.extras.feature/feature.xml b/features/org.eclipse.equinox.p2.extras.feature/feature.xml
index d5adc408b..d69cce961 100644
--- a/features/org.eclipse.equinox.p2.extras.feature/feature.xml
+++ b/features/org.eclipse.equinox.p2.extras.feature/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.extras.feature"
       label="%featureName"
-      version="1.4.2900.qualifier"
+      version="1.4.3000.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
diff --git a/features/org.eclipse.equinox.p2.rcp.feature/feature.xml b/features/org.eclipse.equinox.p2.rcp.feature/feature.xml
index 3da8783e2..a7dcf8d68 100644
--- a/features/org.eclipse.equinox.p2.rcp.feature/feature.xml
+++ b/features/org.eclipse.equinox.p2.rcp.feature/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.rcp.feature"
       label="%featureName"
-      version="1.4.2900.qualifier"
+      version="1.4.3000.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
diff --git a/features/org.eclipse.equinox.p2.sdk/feature.xml b/features/org.eclipse.equinox.p2.sdk/feature.xml
index 884bcfd27..02f3488f7 100644
--- a/features/org.eclipse.equinox.p2.sdk/feature.xml
+++ b/features/org.eclipse.equinox.p2.sdk/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.sdk"
       label="%featureName"
-      version="3.11.2900.qualifier"
+      version="3.11.3000.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
diff --git a/features/org.eclipse.equinox.p2.user.ui/feature.xml b/features/org.eclipse.equinox.p2.user.ui/feature.xml
index ee5457fb2..ba059b135 100644
--- a/features/org.eclipse.equinox.p2.user.ui/feature.xml
+++ b/features/org.eclipse.equinox.p2.user.ui/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.p2.user.ui"
       label="%featureName"
-      version="2.4.2900.qualifier"
+      version="2.4.3000.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
diff --git a/features/org.eclipse.equinox.server.p2/feature.xml b/features/org.eclipse.equinox.server.p2/feature.xml
index 2d2bc2e3d..0f2719038 100644
--- a/features/org.eclipse.equinox.server.p2/feature.xml
+++ b/features/org.eclipse.equinox.server.p2/feature.xml
@@ -2,7 +2,7 @@
 <feature
       id="org.eclipse.equinox.server.p2"
       label="%featureName"
-      version="1.12.1800.qualifier"
+      version="1.12.1900.qualifier"
       provider-name="%providerName"
       license-feature="org.eclipse.license"
       license-feature-version="0.0.0">
-- 
2.51.0

Further information are available in Common Build Issues - Missing version increments.

github-actions · 2025-08-30T14:38:17Z

Test Results

384 files ±0 384 suites ±0 43m 39s ⏱️ + 1m 28s
1 907 tests +2 1 904 ✅ +2 3 💤 ±0 0 ❌ ±0
6 721 runs +6 6 712 ✅ +6 9 💤 ±0 0 ❌ ±0

Results for commit fa1ae12. ± Comparison against base commit 1d14e14.

♻️ This comment has been updated with latest results.

basilevs · 2025-09-04T20:28:12Z

This is ready for review

laeubi · 2025-09-05T05:27:23Z

+			prevService.stop();
+			if (prevService == service) {
+				agentServices.remove(serviceName, prevService);
+				throw new IllegalArgumentException("Double registration"); //$NON-NLS-1$


Is it anywhere mentioned that this is not allowed? Also it would be more an illegalstate than an illegal argument and the message should be more descriptive.

Please see line 99.

if (service instanceof IAgentService) { ((IAgentService) service).start(); }

If double registration was allowed, IAgentService.start() would be invoked twice for the same object.

My original idea was to just unregister existing service, but for the case of double registration, this would leave stopped service in the registry.

I've enhanced the error message with the key, value and explanation.

I've fixed a concurrency bug - the service should only be stopped after removal from the registry.

If double registration was allowed, IAgentService.start() would be invoked twice for the same object.

This is an implementation detail and can of course be avoided, so the question is if anywhere is documented that registering the same service twice is forbidden (at least it was not before)

I've extended the Javadoc.

laeubi · 2025-11-11T12:20:28Z

Build is currently failing, maybe squash / rebase?

…uinox#937

- Ensure that services registered in ProvisionalAgent are disposed when replaced with an alternative. - Ensure services are not exposed to clients until started

basilevs · 2025-11-11T14:55:59Z

Build is currently failing, maybe squash / rebase?

Jenkins build is failing with an unclear error:

Pull request #938 updated
Connecting to https://api.github.com using eclipse-releng-bot/****** (GitHub bot (username/token))
Connecting to https://api.github.com to check permissions of obtain list of basilevs for eclipse-equinox/p2
Loading trusted files from base branch master at 1d14e1496b11b7e2a0c5444be33870a210637482 rather than fa1ae128eabb35a8bfdc628b54de526c192796dd
hudson.plugins.git.GitException: Command "git fetch --tags --force --progress --prune -- origin +refs/heads/*:refs/remotes/origin/*" returned status code 143:
stdout: 
stderr: error: git-remote-https died of signal 15

	at PluginClassLoader for git-client//org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2844)
	at PluginClassLoader for git-client//org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:2189)
	at PluginClassLoader for git-client//org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:638)
	at PluginClassLoader for git//jenkins.plugins.git.GitSCMFileSystem$BuilderImpl.build(GitSCMFileSystem.java:455)
Caused: java.io.IOException
	at PluginClassLoader for git//jenkins.plugins.git.GitSCMFileSystem$BuilderImpl.build(GitSCMFileSystem.java:460)
	at PluginClassLoader for scm-api//jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:316)
	at PluginClassLoader for workflow-multibranch//org.jenkinsci.plugins.workflow.multibranch.SCMBinder.create(SCMBinder.java:109)
	at PluginClassLoader for workflow-job//org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:317)
	at hudson.model.ResourceController.execute(ResourceController.java:101)
	at hudson.model.Executor.run(Executor.java:454)

GitHub has been notified of this commit’s build result

[withMaven] downstreamPipelineTriggerRunListener - Failure to introspect build steps: java.io.IOException: p2/PR-938 #8 did not yet start
[withMaven] downstreamPipelineTriggerRunListener - Failure to introspect build steps: java.io.IOException: p2/PR-938 #8 did not yet start
Finished: FAILURE

How do I fix this?

merks · 2025-11-11T15:09:23Z

This was happening the other week on other servers after the storage migration. 😢

I'll try to kick it again. (The fact that all the GHA tests passed is maybe good enough, but let's try again.)

merks · 2025-11-11T15:19:31Z

It seems stalled just like the previous build. I'm not sure if we need to report it. I asking around...

merks · 2025-11-11T15:52:59Z

Thanks to your rapid response team, i.e., @fredg02, the build is running again. 🚀

basilevs · 2026-03-13T14:49:53Z

I've hit this again while testing on Eclipse 2024-03.

basilevs marked this pull request as draft August 30, 2025 14:34

basilevs closed this Aug 31, 2025

basilevs mentioned this pull request Sep 1, 2025

Fix deadlock #937 #939

Draft

basilevs reopened this Sep 1, 2025

basilevs marked this pull request as ready for review September 1, 2025 13:42

laeubi reviewed Sep 5, 2025

View reviewed changes

basilevs force-pushed the patch-1 branch from ccd2979 to 6dbe7eb Compare September 7, 2025 06:30

basilevs requested a review from laeubi September 9, 2025 20:23

basilevs added 2 commits November 11, 2025 18:25

Do not expose unstarted service, stop started service leak eclipse-eq…

c11ecbc

…uinox#937

Test service disposal eclipse-equinox#937

fa1ae12

- Ensure that services registered in ProvisionalAgent are disposed when replaced with an alternative. - Ensure services are not exposed to clients until started

basilevs force-pushed the patch-1 branch from fe20613 to fa1ae12 Compare November 11, 2025 14:26

merks merged commit 93fb948 into eclipse-equinox:master Nov 11, 2025
12 checks passed

basilevs deleted the patch-1 branch November 11, 2025 16:23

Uh oh!

Conversation

basilevs commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eclipse-equinox-bot commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

basilevs commented Sep 4, 2025

Uh oh!

laeubi Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

basilevs Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

basilevs Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

laeubi Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

basilevs Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

laeubi commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

basilevs commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

merks commented Nov 11, 2025

Uh oh!

merks commented Nov 11, 2025

Uh oh!

merks commented Nov 11, 2025

Uh oh!

Uh oh!

basilevs commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

basilevs commented Aug 30, 2025 •

edited

Loading

eclipse-equinox-bot commented Aug 30, 2025 •

edited

Loading

github-actions Bot commented Aug 30, 2025 •

edited

Loading

basilevs Sep 5, 2025 •

edited

Loading

laeubi commented Nov 11, 2025 •

edited

Loading

basilevs commented Nov 11, 2025 •

edited

Loading

basilevs commented Mar 13, 2026 •

edited

Loading