You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "paulrutter (via GitHub)" <gi...@apache.org> on 2026/08/29 19:51:48 UTC
[PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
paulrutter opened a new pull request, #552:
URL: https://github.com/apache/felix-dev/pull/552
> [!WARNING]
> **Draft / prototype — not for merge.** There is an unresolved licensing question below that blocks this as it stands.
Prototype exploring @tjwatson's suggestion in https://github.com/apache/felix-dev/pull/433#issuecomment-3073468820 that Felix and Equinox should share a cooperative mechanism for the `java.net.URL` factory singletons. Branches off, and targets, #433.
## The problem
`URLHandlers` claims the JVM-wide `URLStreamHandlerFactory` by reflectively swapping a private static field on `java.net.URL`, via `SecureAction.swapStaticFieldIfNotClass`. Obtaining a `MethodHandles.Lookup` trusted enough to do that is the **only remaining reason the framework uses `sun.misc.Unsafe`** — the `staticFieldOffset`/`getObject` steal of `IMPL_LOOKUP`. It also means whichever framework installs itself last wins the singleton, so Felix and Equinox cannot coexist in one JVM without clobbering each other.
To be clear about urgency: this is **not** a JDK 25 blocker. That `Unsafe` block is guarded by `catch (Throwable)` and falls back to `AccessibleObject.setAccessible`, so on JDK 25 it only produces the familiar warning. This is about JDK 26+ and about framework interop.
## What this does
- Vendors the 11 plurl sources from [eclipse-osgi-technology/plurl](https://github.com/eclipse-osgi-technology/plurl) (commit `6581777`) into `org.apache.felix.framework.plurl`. The **only** change is the package rename; every license header and copyright notice is preserved verbatim. This mirrors what Equinox did in https://github.com/eclipse-equinox/equinox/pull/848.
- Adds `PlurlURLHandlers`, adapting `URLHandlers` to `PlurlStreamHandlerFactory` / `PlurlContentHandlerFactory`. Plurl installs one cooperative router through the supported `URL.setURLStreamHandlerFactory` API and routes by asking each registered factory `shouldHandle(Class)` — which maps directly onto what `URLHandlers.getFrameworkFromContext()` already does by hand, and needs no `Unsafe`.
Verified on JDK 25: the framework compiles and its test results are unchanged.
## ⚠️ Licensing — @tjwatson could you confirm?
Every vendored source file declares:
```
SPDX-License-Identifier: EPL-2.0
Copyright (c) 2025 IBM Corporation
```
but plurl's own `LICENSE` file and its `pom.xml` both declare **Apache-2.0**. (Its `NOTICE` also still refers to "slf4j-osgi", which looks like a copy-paste leftover.)
This matters because of where it is being copied *to*. EPL-2.0 is [Category B](https://www.apache.org/legal/resolved.html#category-b) at the ASF and must not be included in an Apache **source** release. Equinox was free to vendor these files because Eclipse projects are EPL-2.0 natively; Felix is not.
Given the repo's LICENSE and pom say Apache-2.0, my guess is the EPL headers are carried over from the original Equinox code rather than intended. **Could you confirm the intended license and, if it is Apache-2.0, correct the source headers upstream?** That would unblock the vendoring approach for Felix and anyone else.
If the headers are intentionally EPL-2.0, then Felix cannot vendor the sources and the options become:
1. consume plurl as a released binary dependency under the Category B rules — which additionally needs plurl published to Maven Central, as it currently has **no release or tag**; or
2. write an Apache-2.0 implementation of the same design in Felix, which works but forks the effort rather than converging on the shared library.
cc @stbischof @laeubi @mattrpav
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531836256
Raised upstream: [plurl#62](https://github.com/eclipse-osgi-technology/plurl/issues/62) with [plurl#63](https://github.com/eclipse-osgi-technology/plurl/pull/63) attached, including the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531575892
> Open an issue against eclipse-osgi-technology/plurl and you can attach your PR there also. That way we can discuss possible API additions there. Thanks for looking at this.
Thanks! Will do and continue the discussion there.
With the proposed patch applied, all unit tests on the Felix side now succeed.
I just have some remaining cleanup todo for Unsafe usage in the meantime.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5499629107
I don't see where you are calling `org.eclipse.osgitech.plurl.Plurl.install(String...)` to get the singleton factories registered with the JVM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "stbischof (via GitHub)" <gi...@apache.org>.
stbischof commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5545733513
Plur snapshot is available.
Ci build is fully working
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5500052221
> > I don't see where you are calling `org.eclipse.osgitech.plurl.Plurl.install(String...)` to get the singleton factories registered with the JVM.
>
> Looking into this, i stopped working on the draft when i noticed the license. Will continue with it now that's resolved 👍🏻
I should mention that the intention was to use a `ServiceLoader` call to lookup the implementation instance of `Plurl`. But I found just doing a `new PlurlImpl()` is easier as long as you don't care to replace the impl with some other `META-INF/services` implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5530800149
You're right and my earlier statement was wrong — apologies. `PlurlImpl.install()` returns early when `checkPlurlProtocol()` finds an installed plurl, and `Plurl.add(..)` goes through the `plurl:` channel, so a second copy registers with the live router rather than replacing it. That rendezvous is exactly what plurl provides, and nothing in the Felix install/registration needed changing.
I've implemented what I think is needed and split it into two commits so the plurl part can be read on its own:
- **[`7f9938c`](https://github.com/apache/felix-dev/commit/7f9938ce896479908213d8c4f7895f9a831df71d)** — the proposed plurl change, touching only the vendored plurl sources
- **[`392c522`](https://github.com/apache/felix-dev/commit/392c52234cd7dfaed69346ae8b462655aa143afc)** — the Felix side that uses it
With both, `URLHandlersTest.urlHandlersWithClassLoaderIsolation` passes and the framework suite is clean.
### Why protocol-level delegation wouldn't help
To answer your question directly: **every Felix framework instance in the JVM uses the same `bundle:` protocol.** Telling plurl "`bundle:` belongs to this factory" doesn't say *which* framework — that's the UUID in the URL host (`<framework-uuid>_<bundleId>`). Equinox encodes the same thing as `<bundleId>.<container.hashCode()>`, but `BundleResourceHandler.openConnection` parses only the bundle id and resolves it against the bound container, so the container part is never consulted for lookup.
What's needed is selection that can see the URL. So:
```java
public interface PlurlFactory {
default boolean shouldHandle(URL url) { return false; }
boolean shouldHandle(Class<?> clazz);
}
```
consulted in `findFactory` **before** walking the call stack. Defaulting to false means existing factories are unaffected. Felix's implementation is then just a UUID comparison against the host.
### Two things that weren't obvious
I had originally proposed only the interface method. It doesn't work on its own, for two reasons I only found by implementing it:
**1. Selection happens while the URL is still empty, and the result is cached.** `PlurlRootURLStreamHandler.parseURL` calls `lookupPlurlStreamHandler(u)`, but in `new URL(spec)` the JDK sets only `protocol` before calling `parseURL` — `host` is populated *by* `parseURL` via `setURL`. So at the moment plurl must choose, there is nothing in the URL to inspect. Worse, that choice is then written to `urlToHandler` for the life of the URL, so a `shouldHandle(URL)` hook is never reached for a freshly parsed URL, which is exactly the failing case.
The commit therefore only records the handler once the URL is usable for selection:
```java
if (!isUsableForSelection(u)) {
// still being parsed; only the call stack is available, and caching this
// choice would pin the URL to a factory picked before the URL was known
return findPlurlStreamHandlerImpl(u);
}
return urlToHandler.get(u, () -> findPlurlStreamHandlerImpl(u));
```
`isUsableForSelection` is just a non-empty-host check today; you may want something stricter or protocol-specific.
**2. `findFactory` iterates `PlurlFactoryHolder`s, not the factories.** Since `PlurlFactoryHolder implements PlurlFactory`, it inherits the default and answers `false` for every holder, so the hook never reaches a real factory. The holder now delegates `shouldHandle(URL)` the same way it already delegates `shouldHandle(Class)`, including the reflective path for factories compiled against a different copy of the plurl package. My first attempt failed precisely because I missed this.
### Two smaller points
- `shouldHandle(URL)` overloads `shouldHandle(Class)`, which makes `shouldHandle(null)` ambiguous — it broke my own test compile. A distinct name would avoid that if you take this upstream.
- Separately: when no factory claims a URL, declining looks safer to me than selecting the first factory added. As it stands framework A can be handed a URL naming framework B, and since Equinox resolves only the bundle id against its bound container, if A also has a bundle with that id it returns A's resource rather than an error. Felix's UUID check turns that into a clean failure, which is why we have a test for it and you likely haven't hit it. Happy to leave that as-is if you'd rather keep the current fallback.
If the shape looks reasonable I'll raise it as a plurl issue or PR — whichever you prefer. And if you'd rather solve it differently, the Felix commit is easy to redo on top of whatever plurl ends up offering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-6036352162
Tried this branch against newer JDKs to see whether anything above 25 breaks: [#570](https://github.com/apache/felix-dev/pull/570) adds **26 and 27** to the matrix (both GA in Temurin — 26.0.2.1 and 27+35 — so these are GA builds, not early access).
All six green, with the framework and TCK steps genuinely running on each rather than being skipped:
| JDK | Felix Framework | OSGi-TCK Framework |
|---|---|---|
| 17 | ✅ | ✅ |
| 21 | ✅ | ✅ |
| 23 | ✅ | ✅ |
| 25 | ✅ | ✅ |
| **26** | ✅ | ✅ |
| **27** | ✅ | ✅ |
[Run 37604709102](https://github.com/apache/felix-dev/actions/runs/37604709102).
To be precise about what this does and does not show: it establishes no regression above 25 for the framework — it builds, its tests pass, and the OSGi Core R8 TCK passes on 26 and 27. It is not evidence of a *fix* for those releases, since nothing here was broken on them to begin with. Only `framework` and `framework.tck` were built; the other modules were filtered out by the path filters, so they remain untested on 26/27.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531595473
Because I've already released a version of Equinox with the current Plurl signatures I will need to attempt to make any new implementation with the new APIs still be able to handle when copies of the old APIs are still in use in the same JVM instance. That may become impossible over time, but I want to at least try to provide some compatibility between versions of plurl.
Otherwise it will become increasingly difficult to be able to update a core framework implementation which hosts other applications which may embed their own version of a framework implementation (Equinox or Felix). Think of a WAR that uses osgi connect to compose their application components using felix or equinox in their WEB-INF/lib folder. And then that WAR being deployed to a server which is implemented with OSGi, such as glashfish(using felix) or liberty(using equinox).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531520613
Open an issue against eclipse-osgi-technology/plurl and you can attach your PR there also. That way we can discuss possible API additions there. Thanks for looking at this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5499933464
> I don't see where you are calling `org.eclipse.osgitech.plurl.Plurl.install(String...)` to get the singleton factories registered with the JVM.
Looking into this, i stopped working on the draft when i noticed the license. Will continue with it now that's resolved 👍🏻
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5464748799
## CI is red on purpose — Apache RAT rejects the vendored sources
Now that stacked PRs get CI (the `pull_request` trigger only accepted `master` as a base, so this PR was getting no build at all until that was fixed on the base branch), the build runs — and fails on all four JDKs in exactly the way that matters:
```
[INFO] Rat check: Summary over all files. Unapproved: 12, unknown: 12, generated: 0, approved: 243 licenses.
[ERROR] Failed to execute goal org.apache.rat:apache-rat-plugin:0.15:check (default)
on project org.apache.felix.framework: Too many files with unapproved license: 12
```
All 12 flagged files are the vendored plurl sources — the 11 `.java` files plus the `README.md`. Nothing else in the framework is flagged.
**This is the licensing question answering itself.** `apache-rat-plugin` is the ASF's own compliance gate, and it will not accept `SPDX-License-Identifier: EPL-2.0` headers in an Apache source tree. It is enforcing the [Category B](https://www.apache.org/legal/resolved.html#category-b) rule automatically. Equinox could vendor these files because Eclipse projects are EPL-2.0 natively; Felix cannot.
The failure is being left in place deliberately. It could be silenced with a RAT exclusion for `plurl/**`, but that would suppress a genuine compliance violation rather than resolve it, and would leave a green tick implying the licensing was settled when it is not. Red is the accurate state: **this branch cannot merge as it stands.**
So the technical side is done and the remaining blocker is purely licensing:
- the framework compiles and its tests are unchanged with plurl vendored and `PlurlURLHandlers` wired in;
- the only thing standing in the way is the header discrepancy — plurl's `LICENSE` and `pom.xml` say Apache-2.0, while every source file says EPL-2.0 with `Copyright (c) 2025 IBM Corporation`.
@tjwatson — if the EPL headers are carried over from the original Equinox code rather than intended, correcting them upstream to match plurl's own Apache-2.0 LICENSE and pom would turn this red build green and unblock the approach for Felix and anyone else wanting to adopt the library. If they are intentionally EPL-2.0, then Felix needs one of the alternatives listed in the PR description instead.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5499563646
Thanks, that's great! I will pull in the latest version and use it verbatim except the package names. Once the JDK 25 PR lands, this one is next on the list.
Once a proper release is out, we can then switch to that artifact instead and remove the vendired sources.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub) via dev" <de...@felix.apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-6580776087
@tjwatson @stbischof since JDK 25 release has just passed the vote and has been released, this one is next on my list to tackle. Let me know if you want me to proceed with vendored plurl sources or rather wait for a proper release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5561534156
> Plur snapshot is available.
>
> Ci build is fully working
I'm proposing a few upstream tweaks so it also covers all Felix needs, see attached PR for plurl.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5466491363
The intention when I moved the source to `osgi-technology` was that I would use the apache header like this:
https://github.com/eclipse-osgi-technology/osgi-test/blob/696b03cd2eaf4e91f7ad0e3d424e4bebb28525ce/org.osgi.test.assertj.feature/src/test/java/org/osgi/test/assertj/feature/AssertionsTest.java#L1-L17
Somehow that was overlooked when setting up the source in https://github.com/eclipse-osgi-technology/plurl
It should be a non-issue to get the headers updated. But what I ultimately wanted to do was have a release out of the osgi-technology project for plurl that would be used by both Equinox and Felix unchanged, in the original package there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5467411088
> The intention when I moved the source to `osgi-technology` was that I would use the apache header like this:
>
> https://github.com/eclipse-osgi-technology/osgi-test/blob/696b03cd2eaf4e91f7ad0e3d424e4bebb28525ce/org.osgi.test.assertj.feature/src/test/java/org/osgi/test/assertj/feature/AssertionsTest.java#L1-L17
>
> Somehow that was overlooked when setting up the source in https://github.com/eclipse-osgi-technology/plurl
>
> It should be a non-issue to get the headers updated. But what I ultimately wanted to do was have a release out of the osgi-technology project for plurl that would be used by both Equinox and Felix unchanged, in the original package there.
Thanks for your quick reply! Let's first see if we can get consensus on the JDK 25 PR, then we can revisit this one.
Would it be possible to get plurl released as a separate artifact, with the right headers, so we can use it without copying the code into Felix?
Apart from that, does this PR make sense to you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5499468337
> Would it be possible to get plurl released as a separate artifact, with the right headers, so we can use it without copying the code into Felix?
I've updated the source headers to use Apache-2.0 with: https://github.com/eclipse-osgi-technology/plurl/pull/45
Note that the source also needed an upstream fix I had put in Equinox with PR: https://github.com/eclipse-osgi-technology/plurl/pull/55
I am not sure how long it will take to get a release out of the osgi-technology project because I have not done a release out of this particular project yet. I'll ask existing members of the project about what steps are needed to get a release done. But I don't want that to block you on progress. With the current state of the source I think it should be fine for you to proceed with copying in the source now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531786684
One correction before I open the plurl issue: the `shouldHandle(URL)` shape I proposed doesn't work, and it took running plurl's own test suite to see why.
`PlurlRootURLStreamHandler.parseURL` pins the selected handler onto the URL through the private `java.net.URL.handler` field. Wherever that field is writable — plurl's own test build passes `--add-opens java.base/java.net`, and the Felix framework bundle declares `Add-opens: java.base/java.net` — the swap succeeds, plurl steps aside, and `openConnection` never comes back through it. So a URL-based hook is unreachable in exactly the deployments that need it. My Felix test passed only because that JVM had no add-opens, which is the opposite of reassuring.
Selection has to be decidable at the moment plurl chooses, i.e. mid-parse, when the URL has no host yet. The spec does exist then:
```java
default boolean shouldHandle(String protocol, String spec) { return false; }
```
consulted in `findFactory` before the call stack is walked, with `PlurlFactoryHolder` delegating it the way it already delegates `shouldHandle(Class)` — without that, `findFactory` only ever asks the holders, which answer with the interface default.
On your compatibility point: the signature uses only `java.lang` types, so the reflective path finds it on a factory compiled against a different copy of the package, and a factory that predates the method is tolerated rather than throwing — the lookup is cached, since it sits on the URL parsing path. Existing factories are unaffected either way. To be straight about the limit though: this only helps when the *installed* router is new enough to consult it. If an older plurl wins the install, Felix falls back to today's behaviour, and nothing on the factory side can change that.
- [`81442bc`](https://github.com/apache/felix-dev/commit/81442bc1bd) — the plurl change, vendored copy only
- [`07d572a`](https://github.com/apache/felix-dev/commit/07d572a503) — the Felix side, now claiming by UUID read from the spec
51/51 in the plurl repo, including a new `PlurlURLSelectionTest` that fails without the patch (both URLs served by the first-added factory) and passes with it. Felix's suite is unchanged. I'll raise the issue and PR upstream with that test included.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5531912431
Also did the `Unsafe` cleanup I had left over: [`33cf2ff`](https://github.com/apache/felix-dev/commit/33cf2ff539).
With plurl installing the JVM factories, `URLHandlers` is never instantiated — its private constructor was the only caller of the reflective field clearing. Everything reachable only from there was dead, including the whole cross-classloader rendezvous it existed to support (`registerFrameworkListsForContextSearch` published the framework list to a `URLHandlers` in another classloader, which plurl now does properly). The class keeps only the framework registry that `URLHandlersBundleStreamHandler` needs to find the owner of a caller when a handler isn't bound to one. `SecureAction` loses `swapStaticFieldIfNotClass` and `flush`, which had no callers left. Net −794 lines.
That removes every reflective write to a `java.net` static field. To be complete: one `sun.misc.Unsafe` use remains, in `SecureAction.getAccessor`. It's a different concern — the `setAccessible` strategy for deep reflection, still needed by `addURLToURLClassLoader` when installing framework extension bundles — so I left it alone rather than fold an unrelated change in here.
Suite is at the pre-existing baseline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5528461596
> Keep the guarantee with a Felix-side cross-copy UUID registry — which reintroduces reflection into exactly the mechanism plurl is meant to replace, since each class loader has its own copy of both URLHandlers and the plurl classes and there's no shared rendezvous to publish to.
I am a bit confused by this because there is a unit test in plurl that mimics a copy scenario and a separate class loader (proxy) scenario. The intent of Plurl is not only can the users have their own copy loaded by a difference class loader, but their own copy could live in a completely different package name. For example. see the test that has a copy of Plurl APIs at
https://github.com/eclipse-osgi-technology/plurl/tree/main/src/test/java/org/eclipse/osgitech/plurl/test/copy
In this case it reflects on agreed upon method names from the Plurl APIs. This way, as long as the copies mirror the upstream Plurl APIs then they can intermix in the same JVM. This way Felix and Equinox can both run in the same JVM and still multiplex on the protocols even if they copied the plurl API/impl into their own felix and equinox packages.
Can you go into more details on what plurl changes you need to get the `bundle` protocol to work in Felix. Do you need some alternative way to tell plurl about your protocols so that we delegate to the factory for the protocol differently than the default?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5500469901
Thanks for sorting the headers — `apache-rat` is satisfied now, which was blocking the build entirely. I've pushed a substantial rework; summary below, with one design question at the end.
I refreshed the vendored sources from upstream, so they include the Apache-2.0 headers from eclipse-osgi-technology/plurl#45 and the fix from eclipse-osgi-technology/plurl#55. Only the package rename is applied. And `new PlurlImpl()` rather than the `ServiceLoader`, as you suggested.
### Both of your review points were real defects
- **`Plurl.install(..)` was never called.** Since the static `Plurl.add(..)` helpers operate through a `plurl:` URL, without installing the router first they fail with *unknown protocol: plurl*. Nothing in the draft exercised the adapter, so it compiled and the tests passed while being dead code.
- **`shouldHandle` was too loose.** It only checked that a class came from *some* Felix bundle class loader, so with two frameworks in one JVM either could answer for the other's bundles. It now also requires the owning framework to match, as your `EquinoxBundle`/container check does.
### Plurl is now the mechanism, not an addition alongside it
- `URLHandlers` no longer swaps the `java.net.URL`/`URLConnection` static fields, and its singleton is not constructed. I had planned to keep that as a fallback, but the `catch` in your `SystemBundleActivator` changed my mind: a fallback keeps the `sun.misc.Unsafe` path alive, which is the thing we're trying to delete. Felix now behaves as Equinox does — if registration fails it logs and that framework contributes no URL handlers.
- Removing the swap turned out to be **necessary**, not just tidy. With `URLHandlers` taking the singletons first, plurl found them occupied and required `--add-opens java.base/java.net=ALL-UNNAMED`. Installing into a clean JVM uses the supported API and needs no flag.
- `URLHandlersBundleStreamHandler` and `URLHandlersStreamHandlerProxy` now extend `PlurlStreamHandlerBase`, mirroring your changes to `BundleResourceHandler` and `URLStreamHandlerProxy`.
- The router is installed once per JVM and reference counted. Uninstalling per framework tore it down while another framework was still registered.
- Added a test that starts a framework and asserts registration actually happened, that the `plurl:` protocol resolves, and that `shouldHandle` doesn't claim classes outside the framework.
On Linux the framework suite is 121 tests with a single error, which is the question below.
### Routing a URL re-parsed outside any bundle
`URLHandlersTest.urlHandlersWithClassLoaderIsolation` loads a second copy of the whole framework — and therefore of the vendored plurl classes — in a separate class loader, each with its own framework. It then does the equivalent of:
```java
URL url = bundle.getEntry("...");
new URL(url.toExternalForm()).openStream(); // called from a plain test class
```
That second `new URL(..)` is re-parsed **without** an explicit handler, from a class not loaded by any bundle. There is no bundle frame anywhere on the stack, so `shouldHandle` cannot attribute it and plurl has nothing to route on.
Note I deliberately do *not* bind the `bundle:` handler to one framework the way you bind yours to the container. The JVM caches one handler per protocol, so a pinned handler also gets used for another framework's `bundle:` URLs. Instead the framework is resolved per call from the UUID in the URL host, via `URLHandlers.getFrameworkFromContext(uuid)` — which only sees its own copy's frameworks. Previously a second copy reflectively called `registerFrameworkListsForContextSearch` on whichever copy owned the JVM factory, so UUIDs resolved across class loaders. With plurl there is no "root `URLHandlers`" to find, so that rendezvous is gone.
I initially assumed Equinox must solve this and Felix was missing something. Reading `BundleResourceHandler` properly, it's the other way round:
```java
String host = url.getHost(); // "<bundleId>.<container.hashCode()>"
bundleID = parseBundleIDFromURLHost(host); // parses ONLY the bundle id
Module module = getModule(bundleID); // -> container.getModule(id), the BOUND container
```
The container is encoded in the host by `createURLHostForBundleID`, but `openConnection` parses only the bundle id and resolves it against whichever container the handler was constructed with; the container part is used for `equals`/`hashCode`/`hostsEqual`/`sameFile`, never to locate the owner. So in this scenario Equinox would either return the resource with that id **from the wrong container**, or fail with `URL_NO_BUNDLE_FOUND`. It relies on `shouldHandle` having routed correctly, with no fallback when attribution is impossible.
Felix's UUID lookup is a stronger guarantee, which is why there's a test for it here and no equivalent upstream. So the question isn't how Equinox does it, but:
**Would plurl consider routing a `URL` to its owning factory, not only a calling `Class`?**
A `boolean shouldHandle(URL)` on `PlurlStreamHandlerFactory`, consulted when call stack attribution yields nothing, would let each framework claim its own URLs by inspecting the host. Both frameworks would then be correct for URLs re-parsed outside a bundle, and Equinox could tighten its behaviour if it wanted to.
Without that, the Felix-side options are:
1. Match Equinox — bind the handler to one framework and accept that cross-framework re-parsing resolves against the wrong one. Converges with upstream, but drops behaviour Felix has today and means weakening or removing that test.
2. Keep the guarantee with a Felix-side cross-copy UUID registry — which reintroduces reflection into exactly the mechanism plurl is meant to replace, since each class loader has its own copy of both `URLHandlers` and the plurl classes and there's no shared rendezvous to publish to.
I'd rather not do 1 silently. Happy to raise the API idea as a plurl issue with a concrete proposal if you think it's reasonable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "tjwatson (via GitHub)" <gi...@apache.org>.
tjwatson commented on code in PR #552:
URL: https://github.com/apache/felix-dev/pull/552#discussion_r3907938699
##########
framework/src/main/java/org/apache/felix/framework/PlurlURLHandlers.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.framework;
+
+import java.io.IOException;
+import java.net.ContentHandler;
+import java.net.URLStreamHandler;
+
+import org.apache.felix.framework.plurl.Plurl;
+import org.apache.felix.framework.plurl.PlurlContentHandlerFactory;
+import org.apache.felix.framework.plurl.PlurlStreamHandlerFactory;
+
+/**
+ * <p>
+ * PROTOTYPE (FELIX-6759): adapts Felix' {@link URLHandlers} onto the plurl
+ * multiplexing URL factories.
+ * </p>
+ * <p>
+ * {@code URLHandlers} currently claims the JVM-wide {@code java.net.URL} stream
+ * handler factory by reflectively swapping a private static field
+ * ({@code SecureAction.swapStaticFieldIfNotClass}). Obtaining a
+ * {@code MethodHandles.Lookup} trusted enough to do that is the sole reason the
+ * framework still uses {@code sun.misc.Unsafe}, and it means whichever framework
+ * installs itself last wins the singleton - so Felix and Equinox cannot coexist in
+ * one JVM without clobbering each other.
+ * </p>
+ * <p>
+ * Plurl instead installs one cooperative router through the supported
+ * {@code URL.setURLStreamHandlerFactory} API and lets any number of parties
+ * register with it. Each registered factory answers {@link #shouldHandle(Class)}
+ * to say whether a given calling class belongs to it; plurl walks the call stack
+ * and routes accordingly. That maps directly onto what
+ * {@link URLHandlers#getFrameworkFromContext()} already does.
+ * </p>
+ * <p>
+ * See {@code org/apache/felix/framework/plurl/README.md} for the provenance of the
+ * vendored plurl sources and the <b>unresolved licensing question</b> that currently
+ * blocks this approach.
+ * </p>
+ */
+class PlurlURLHandlers implements PlurlStreamHandlerFactory, PlurlContentHandlerFactory
+{
+ private final URLHandlers m_delegate;
+
+ PlurlURLHandlers(URLHandlers delegate)
+ {
+ m_delegate = delegate;
+ }
+
+ /**
+ * Registers this framework's handlers with the plurl router, installing the
+ * router first if nobody has yet.
+ */
+ static PlurlURLHandlers install(URLHandlers delegate) throws IOException
+ {
+ PlurlURLHandlers handlers = new PlurlURLHandlers(delegate);
+ Plurl.add((PlurlStreamHandlerFactory) handlers);
+ Plurl.add((PlurlContentHandlerFactory) handlers);
+ return handlers;
+ }
+
+ /**
+ * Unregisters this framework's handlers, leaving the router in place for any
+ * other framework instance still using it.
+ */
+ void uninstall() throws IOException
+ {
+ Plurl.remove((PlurlStreamHandlerFactory) this);
+ Plurl.remove((PlurlContentHandlerFactory) this);
+ }
+
+ /**
+ * Tells plurl whether the given calling class belongs to this framework.
+ * <p>
+ * This replaces the call stack walking URLHandlers does today: rather than
+ * inspecting the stack itself to work out which framework owns the caller, the
+ * router asks each registered factory about a single candidate class.
+ */
+ @Override
+ public boolean shouldHandle(Class<?> clazz)
+ {
+ if (clazz == null)
+ {
+ return false;
+ }
+ ClassLoader loader = clazz.getClassLoader();
+ if (loader == null)
+ {
+ return false;
+ }
+ String name = loader.getClass().getName();
+ return name.startsWith("org.apache.felix.framework.BundleWiringImpl$BundleClassLoader")
+ || name.startsWith("org.apache.felix.framework.ModuleImpl$ModuleClassLoader")
+ || name.equals("org.apache.felix.framework.searchpolicy.ContentClassLoader");
Review Comment:
For Equinox I do an additional check to make sure the class loader is for a bundle that is actually installed in this framework instance. I assume you would want the same thing here. Otherwise one felix framework may handle lookups for a bundle resolved in another felix framework instance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5532528269
Added the silent-degradation warning, and the upstream half that makes it possible — [plurl#63](https://github.com/eclipse-osgi-technology/plurl/pull/63) now has a second commit.
The problem: the plurl copy that wins the install in a JVM is the one that routes, and it may be older than the copy this framework brought. We register successfully and are then silently misrouted, with no way to tell — `bundle:` URLs can only be attributed by the UUID they carry, so without `shouldHandle(protocol, spec)` they go to whichever factory registered first.
Upstream now answers a `plurlCapabilities` query. An implementation predating it rejects the operation with `IOException`, so absence of an answer is the answer and needs no cooperation from already-released plurl.
- [`1947fc4`](https://github.com/apache/felix-dev/commit/1947fc465e) — vendored plurl change
- [`45b65b6`](https://github.com/apache/felix-dev/commit/45b65b665c) — Felix warns at startup when the installed router can't route `bundle:` URLs, instead of leaving it to surface later as a failed resource lookup. The test pins the vendored copy's capability, so re-vendoring an older plurl fails there rather than quietly turning the warning on for everyone.
Also [`cdd41d1`](https://github.com/apache/felix-dev/commit/cdd41d1244): the vendored plurl sources carry `Copyright (c) Contributors to the Eclipse Foundation` and were credited in no legal file — not NOTICE, not LICENSE, not DEPENDENCIES. Added the NOTICE entry alongside the existing OSGi Alliance one, which is the same case; no LICENSE change needed since plurl is ALv2 too. RAT doesn't catch this, it only checks source headers.
Framework suite at the pre-existing baseline. @tjwatson no action needed on the Felix commits, but the capability commit upstream is take-it-or-leave-it — the selection commit stands on its own without it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on code in PR #552:
URL: https://github.com/apache/felix-dev/pull/552#discussion_r4280293798
##########
framework/src/main/java/org/apache/felix/framework/PlurlURLHandlers.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.framework;
+
+import java.io.IOException;
+import java.net.ContentHandler;
+import java.net.URLStreamHandler;
+
+import org.apache.felix.framework.plurl.Plurl;
+import org.apache.felix.framework.plurl.PlurlContentHandlerFactory;
+import org.apache.felix.framework.plurl.PlurlStreamHandlerFactory;
+
+/**
+ * <p>
+ * PROTOTYPE (FELIX-6759): adapts Felix' {@link URLHandlers} onto the plurl
+ * multiplexing URL factories.
+ * </p>
+ * <p>
+ * {@code URLHandlers} currently claims the JVM-wide {@code java.net.URL} stream
+ * handler factory by reflectively swapping a private static field
+ * ({@code SecureAction.swapStaticFieldIfNotClass}). Obtaining a
+ * {@code MethodHandles.Lookup} trusted enough to do that is the sole reason the
+ * framework still uses {@code sun.misc.Unsafe}, and it means whichever framework
+ * installs itself last wins the singleton - so Felix and Equinox cannot coexist in
+ * one JVM without clobbering each other.
+ * </p>
+ * <p>
+ * Plurl instead installs one cooperative router through the supported
+ * {@code URL.setURLStreamHandlerFactory} API and lets any number of parties
+ * register with it. Each registered factory answers {@link #shouldHandle(Class)}
+ * to say whether a given calling class belongs to it; plurl walks the call stack
+ * and routes accordingly. That maps directly onto what
+ * {@link URLHandlers#getFrameworkFromContext()} already does.
+ * </p>
+ * <p>
+ * See {@code org/apache/felix/framework/plurl/README.md} for the provenance of the
+ * vendored plurl sources and the <b>unresolved licensing question</b> that currently
+ * blocks this approach.
+ * </p>
+ */
+class PlurlURLHandlers implements PlurlStreamHandlerFactory, PlurlContentHandlerFactory
+{
+ private final URLHandlers m_delegate;
+
+ PlurlURLHandlers(URLHandlers delegate)
+ {
+ m_delegate = delegate;
+ }
+
+ /**
+ * Registers this framework's handlers with the plurl router, installing the
+ * router first if nobody has yet.
+ */
+ static PlurlURLHandlers install(URLHandlers delegate) throws IOException
+ {
+ PlurlURLHandlers handlers = new PlurlURLHandlers(delegate);
+ Plurl.add((PlurlStreamHandlerFactory) handlers);
+ Plurl.add((PlurlContentHandlerFactory) handlers);
+ return handlers;
+ }
+
+ /**
+ * Unregisters this framework's handlers, leaving the router in place for any
+ * other framework instance still using it.
+ */
+ void uninstall() throws IOException
+ {
+ Plurl.remove((PlurlStreamHandlerFactory) this);
+ Plurl.remove((PlurlContentHandlerFactory) this);
+ }
+
+ /**
+ * Tells plurl whether the given calling class belongs to this framework.
+ * <p>
+ * This replaces the call stack walking URLHandlers does today: rather than
+ * inspecting the stack itself to work out which framework owns the caller, the
+ * router asks each registered factory about a single candidate class.
+ */
+ @Override
+ public boolean shouldHandle(Class<?> clazz)
+ {
+ if (clazz == null)
+ {
+ return false;
+ }
+ ClassLoader loader = clazz.getClassLoader();
+ if (loader == null)
+ {
+ return false;
+ }
+ String name = loader.getClass().getName();
+ return name.startsWith("org.apache.felix.framework.BundleWiringImpl$BundleClassLoader")
+ || name.startsWith("org.apache.felix.framework.ModuleImpl$ModuleClassLoader")
+ || name.equals("org.apache.felix.framework.searchpolicy.ContentClassLoader");
Review Comment:
Agreed, and that check is in place: `shouldHandle(Class)` requires the owning framework to match, not just that the class came from some Felix bundle.
```java
Bundle bundle = ((BundleReference) loader).getBundle();
return (bundle instanceof BundleImpl)
&& (((BundleImpl) bundle).getFramework() == m_felix);
```
Two framework instances in the same JVM both load through a `BundleWiringImpl.BundleClassLoader`, so without the `getFramework() == m_felix` comparison one framework would answer lookups for a bundle resolved in the other — the case you describe. The javadoc on the method calls that out so it does not get simplified away later.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - Remove sun.misc.Unsafe - use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on code in PR #552:
URL: https://github.com/apache/felix-dev/pull/552#discussion_r4280293798
##########
framework/src/main/java/org/apache/felix/framework/PlurlURLHandlers.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.framework;
+
+import java.io.IOException;
+import java.net.ContentHandler;
+import java.net.URLStreamHandler;
+
+import org.apache.felix.framework.plurl.Plurl;
+import org.apache.felix.framework.plurl.PlurlContentHandlerFactory;
+import org.apache.felix.framework.plurl.PlurlStreamHandlerFactory;
+
+/**
+ * <p>
+ * PROTOTYPE (FELIX-6759): adapts Felix' {@link URLHandlers} onto the plurl
+ * multiplexing URL factories.
+ * </p>
+ * <p>
+ * {@code URLHandlers} currently claims the JVM-wide {@code java.net.URL} stream
+ * handler factory by reflectively swapping a private static field
+ * ({@code SecureAction.swapStaticFieldIfNotClass}). Obtaining a
+ * {@code MethodHandles.Lookup} trusted enough to do that is the sole reason the
+ * framework still uses {@code sun.misc.Unsafe}, and it means whichever framework
+ * installs itself last wins the singleton - so Felix and Equinox cannot coexist in
+ * one JVM without clobbering each other.
+ * </p>
+ * <p>
+ * Plurl instead installs one cooperative router through the supported
+ * {@code URL.setURLStreamHandlerFactory} API and lets any number of parties
+ * register with it. Each registered factory answers {@link #shouldHandle(Class)}
+ * to say whether a given calling class belongs to it; plurl walks the call stack
+ * and routes accordingly. That maps directly onto what
+ * {@link URLHandlers#getFrameworkFromContext()} already does.
+ * </p>
+ * <p>
+ * See {@code org/apache/felix/framework/plurl/README.md} for the provenance of the
+ * vendored plurl sources and the <b>unresolved licensing question</b> that currently
+ * blocks this approach.
+ * </p>
+ */
+class PlurlURLHandlers implements PlurlStreamHandlerFactory, PlurlContentHandlerFactory
+{
+ private final URLHandlers m_delegate;
+
+ PlurlURLHandlers(URLHandlers delegate)
+ {
+ m_delegate = delegate;
+ }
+
+ /**
+ * Registers this framework's handlers with the plurl router, installing the
+ * router first if nobody has yet.
+ */
+ static PlurlURLHandlers install(URLHandlers delegate) throws IOException
+ {
+ PlurlURLHandlers handlers = new PlurlURLHandlers(delegate);
+ Plurl.add((PlurlStreamHandlerFactory) handlers);
+ Plurl.add((PlurlContentHandlerFactory) handlers);
+ return handlers;
+ }
+
+ /**
+ * Unregisters this framework's handlers, leaving the router in place for any
+ * other framework instance still using it.
+ */
+ void uninstall() throws IOException
+ {
+ Plurl.remove((PlurlStreamHandlerFactory) this);
+ Plurl.remove((PlurlContentHandlerFactory) this);
+ }
+
+ /**
+ * Tells plurl whether the given calling class belongs to this framework.
+ * <p>
+ * This replaces the call stack walking URLHandlers does today: rather than
+ * inspecting the stack itself to work out which framework owns the caller, the
+ * router asks each registered factory about a single candidate class.
+ */
+ @Override
+ public boolean shouldHandle(Class<?> clazz)
+ {
+ if (clazz == null)
+ {
+ return false;
+ }
+ ClassLoader loader = clazz.getClassLoader();
+ if (loader == null)
+ {
+ return false;
+ }
+ String name = loader.getClass().getName();
+ return name.startsWith("org.apache.felix.framework.BundleWiringImpl$BundleClassLoader")
+ || name.startsWith("org.apache.felix.framework.ModuleImpl$ModuleClassLoader")
+ || name.equals("org.apache.felix.framework.searchpolicy.ContentClassLoader");
Review Comment:
Agreed, and that check is in place: `shouldHandle(Class)` requires the owning framework to match, not just that the class came from some Felix bundle.
```java
Bundle bundle = ((BundleReference) loader).getBundle();
return (bundle instanceof BundleImpl)
&& (((BundleImpl) bundle).getFramework() == m_felix);
```
Two framework instances in the same JVM both load through a `BundleWiringImpl.BundleClassLoader`, so without the `getFramework() == m_felix` comparison one framework would answer lookups for a bundle resolved in the other — the case you describe. The javadoc on the method calls that out so it does not get simplified away later.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5500355448
Thanks — the Apache-2.0 headers unblocked this, and `apache-rat` is happy now. I've pushed a substantial rework, and I have one design question at the end I'd value your view on.
Both of your review points were real defects, not just omissions:
- **`Plurl.install(..)` was never called.** Since the static `Plurl.add(..)` helpers operate through a `plurl:` URL, without installing the router first they fail with *unknown protocol: plurl*. Nothing in the prototype exercised the adapter, so it compiled and the tests passed while being dead code.
- **`shouldHandle` was too loose.** It only checked that a class came from *some* Felix bundle class loader, so with two frameworks in one JVM either could answer for the other's bundles. It now also requires the owning framework to match, as your `EquinoxBundle`/container check does.
Beyond that, plurl is now the mechanism rather than something bolted alongside, following Equinox:
- `URLHandlers` no longer swaps the `java.net.URL`/`URLConnection` static fields, and its singleton is not constructed. I had considered keeping that as a fallback, but your `catch` in `SystemBundleActivator` convinced me otherwise — a fallback keeps the `sun.misc.Unsafe` path alive, which is the thing we're trying to delete.
- Removing the swap turned out to be *necessary*, not just tidy: with `URLHandlers` taking the singletons first, plurl found them occupied and required `--add-opens java.base/java.net=ALL-UNNAMED`. Installing into a clean JVM uses the supported API and needs no flag.
- `URLHandlersBundleStreamHandler` and `URLHandlersStreamHandlerProxy` now extend `PlurlStreamHandlerBase`, mirroring your changes to `BundleResourceHandler` and `URLStreamHandlerProxy`.
- The router is installed once per JVM and reference counted — uninstalling per framework tore it down while another framework was still registered.
- `new PlurlImpl()` as you suggested, rather than the `ServiceLoader`.
The framework suite on JDK 25 is at 121 tests, 9 failures, 8 of which are pre-existing platform-specific ones unrelated to this. There is one left, and it's a design question rather than a bug:
### Routing a URL re-parsed outside any bundle
`URLHandlersTest.urlHandlersWithClassLoaderIsolation` loads a second copy of the whole framework (and therefore of the vendored plurl classes) in a separate class loader, and each copy starts its own framework. The test then does the equivalent of:
```java
URL url = bundle.getEntry("...");
new URL(url.toExternalForm()).openStream(); // called from a plain test class
```
That second `new URL(..)` is re-parsed **without** an explicit handler, from a class that is not loaded by any bundle. So `shouldHandle` cannot attribute it to a framework and plurl has nothing to route on.
Falling through, the `bundle:` handler has to resolve the framework itself from the UUID in the URL's host. It can only see its own copy's frameworks, so when the URL belongs to the framework in the other class loader the lookup fails. Previously `URLHandlers` bridged this: a second copy reflectively called `registerFrameworkListsForContextSearch` on whichever copy owned the JVM factory, so a UUID could be resolved across class loaders. With plurl there is no "root `URLHandlers`" to find, so that rendezvous is gone.
Note I deliberately do **not** bind the `bundle:` handler to one framework the way you bind yours to the container: the JVM caches one handler per protocol, so a pinned handler also gets used for another framework's `bundle:` URLs and then fails to resolve. That's what forced the UUID lookup.
So: **how should a URL re-parsed from outside any bundle be routed to the owning framework, when several copies of plurl are live in different class loaders?** Options I can see:
1. plurl gains a way to route a *URL* (not just a calling class), or to enumerate registered factories, so the owning factory can be found.
2. Felix keeps its own cross-copy UUID registry — but that reintroduces reflection into the mechanism plurl is meant to replace.
3. It's accepted as out of scope, and the test's expectation changes.
Does Equinox have an equivalent case? I suspect it doesn't hit this because its callers are bundle classes that plurl can attribute, so the question may not have come up. If you think (1) is reasonable I'm happy to raise it upstream with a concrete API suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5500454148
Correction to my previous comment: I said I suspected Equinox simply does not hit this case. Having read `BundleResourceHandler` properly, that was the wrong way round — Equinox does not have the problem because it does not attempt the resolution at all, and Felix currently guarantees something stronger.
`openConnection` there does:
```java
String host = url.getHost(); // "<bundleId>.<container.hashCode()>"
bundleID = parseBundleIDFromURLHost(host); // parses ONLY the bundle id
Module module = getModule(bundleID); // -> container.getModule(id), the BOUND container
```
The framework identity is encoded in the host by `createURLHostForBundleID(container, bundleId)`, but `openConnection` parses only the bundle id and resolves it against whichever container the handler instance was constructed with. The container part of the host is used for `equals`/`hashCode`/`hostsEqual`/`sameFile` comparisons, never to locate the owning container.
So for the case Felix's `urlHandlersWithClassLoaderIsolation` covers — a `bundle:` URL re-parsed by a caller that is not in any bundle, with two frameworks live — Equinox would either return the resource with that bundle id **from the wrong container**, or fail with `URL_NO_BUNDLE_FOUND`. It depends entirely on plurl's `shouldHandle` having routed to the right factory, with no fallback when attribution is impossible.
Felix's `URLHandlers.getFrameworkFromContext(uuid)` instead resolves the owning framework from the UUID in the URL, and previously did so across class loaders via the `registerFrameworkListsForContextSearch` bridge. That is a stronger guarantee, and it is why Felix has a test for it and there is no equivalent upstream.
That changes what I am actually asking. Not "how does Equinox do this" — it doesn't — but:
**Would plurl consider an API for routing a `URL` to its owning factory, rather than only routing a calling `Class`?**
Something like a `PlurlStreamHandlerFactory` callback along the lines of `boolean shouldHandle(URL)`, consulted when call stack attribution yields nothing, would let each framework claim its own URLs by inspecting the host. Both frameworks would then behave correctly for URLs re-parsed outside a bundle, and Equinox could tighten its behaviour too if it wanted to.
Without that, the options on the Felix side are:
1. Match Equinox: bind the handler to one framework and accept that cross-framework re-parsing resolves against the wrong one. Converges with upstream, but is a regression for Felix and means weakening or removing that test.
2. Keep the current guarantee with a Felix-side cross-copy UUID registry. That reintroduces reflection into exactly the mechanism plurl is meant to replace, since each class loader has its own copy of both `URLHandlers` and the plurl classes, and there is no shared rendezvous to publish to.
I would rather not do 1 silently, since it drops behaviour Felix has today. Happy to raise (1) as a plurl issue with a concrete API proposal if you think it is reasonable.
For the record the branch is otherwise in good shape: on Linux the framework suite is 121 tests with this as the only error, and `apache-rat` is satisfied now the headers are Apache-2.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5530989103
You're right, my earlier statement was wrong — `install()` returns early when `checkPlurlProtocol()` finds an installed plurl, so a second copy registers with the live router rather than replacing it. Nothing in the Felix registration needed changing.
To answer your question: protocol-level delegation wouldn't help, because **every Felix framework instance uses the same `bundle:` protocol**. The owner is identified by the framework UUID in the URL host, so selection needs to see the URL. Implemented and split so the plurl part can be lifted on its own:
- [`7f9938c`](https://github.com/apache/felix-dev/commit/7f9938ce896479908213d8c4f7895f9a831df71d) — `PlurlFactory` gains `default boolean shouldHandle(URL)` returning false; consulted during selection and delegated through `PlurlFactoryHolder`
- [`5952c24`](https://github.com/apache/felix-dev/commit/5952c24986e4e2cfa813a05533221f7e479c0a69) — narrows it so only a factory that *claims* a URL can act, keeping the per-URL pinning
- [`392c522`](https://github.com/apache/felix-dev/commit/392c52234cd7dfaed69346ae8b462655aa143afc) — Felix side: claim `bundle:` URLs by framework UUID
`URLHandlersTest.urlHandlersWithClassLoaderIsolation` now passes.
Three things worth knowing, all found by implementing it:
1. **The interface method alone is inert.** Selection happens in `parseURL`, where the JDK has set only `protocol` — `host` is populated *by* `parseURL` — so there's nothing to inspect, and that choice is then cached for the URL's lifetime. `PlurlFactoryHolder` also had to delegate the new method, or the default answers for every holder and never reaches a factory.
2. **Nothing changes unless a factory opts in.** My first attempt skipped the `urlToHandler` record while the URL was unparsed, which would have broken Equinox: `BundleResourceHandler` keeps mutable per-URL state (`bundleEntry`, cleared in `parseURL`, used as a fast path in `openConnection`) that assumes one handler per URL. `5952c24` instead records as before and lets only a claiming factory correct it once. Both Equinox factories declare only `shouldHandle(Class)`, so they never claim and behave identically.
3. **`shouldHandle(URL)` overloads `shouldHandle(Class)`**, making `shouldHandle(null)` ambiguous — it broke my own test compile. A distinct name would avoid that upstream.
Separately: when no factory claims a URL, declining may be safer than selecting the first factory added, since framework A can be handed a URL naming framework B. Felix's UUID check makes that a clean failure; Equinox resolves only the bundle id against its bound container, so it would answer with its own resource. Happy to leave the fallback as-is if you'd rather.
Glad to raise this as a plurl issue/PR, squashed however you prefer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6759 - PROTOTYPE: use plurl for URL handler factory multiplexing [felix-dev]
Posted by "paulrutter (via GitHub)" <gi...@apache.org>.
paulrutter commented on PR #552:
URL: https://github.com/apache/felix-dev/pull/552#issuecomment-5530973226
Follow-up: I've narrowed the plurl change so that **nothing changes for a factory that doesn't implement `shouldHandle(URL)`** — including both Equinox factories, which declare only `shouldHandle(Class)`.
- **[`5952c24`](https://github.com/apache/felix-dev/commit/5952c24986e4e2cfa813a05533221f7e479c0a69)** — the refinement (plurl only)
I checked what my first version would have done to Equinox and wasn't comfortable with it. Skipping the `urlToHandler` record while the URL was unparsed meant selection re-ran at first use, which breaks an assumption `BundleResourceHandler` makes: it keeps mutable per-URL state — `bundleEntry`, cleared in `parseURL` and used as a fast path in `openConnection` — which only works if one handler serves a URL from parse through to use. It could also have routed to a different container when the call stack at first use differs from the one at parse time. Neither is something Equinox should have to accept for a hook it doesn't use.
The record is now written during `parseURL` exactly as before, and only a factory that *positively claims* the URL may correct it, once, after which it stays pinned as today:
- `findFactory` is restored to its original form, so URL-based selection no longer touches the general path.
- `findClaimedPlurlStreamHandler` answers **only** when a factory claims the URL — no call stack inspection, no fallback to the first factory, so it returns null unless an owner identifies the URL as its own.
- `URLToHandler.replace` updates the record for that URL.
So the opt-in surface is exactly the new default method. `URLHandlersTest.urlHandlersWithClassLoaderIsolation` still passes and the framework suite is unchanged.
For reference the three plurl-side commits, in order:
1. [`7f9938c`](https://github.com/apache/felix-dev/commit/7f9938ce896479908213d8c4f7895f9a831df71d) — `shouldHandle(URL)`, consulted in selection, delegated through `PlurlFactoryHolder`
2. [`5952c24`](https://github.com/apache/felix-dev/commit/5952c24986e4e2cfa813a05533221f7e479c0a69) — narrowed to opt-in, preserving the per-URL pinning
3. [`392c522`](https://github.com/apache/felix-dev/commit/392c52234cd7dfaed69346ae8b462655aa143afc) — the Felix side, claiming `bundle:` URLs by framework UUID
If you'd prefer them squashed into one commit for lifting upstream, happy to do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org