Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access restrictions of package-private elements in dependencies is not respected #1241

Open
HannesWell opened this issue Apr 17, 2024 · 10 comments

Comments

@HannesWell
Copy link
Member

PDE does not (or at least does not configure JDT to) issue compiler errors if a class accesses package-private elements of anther class that resides in another bundle in a package with the same name, although such access fails at runtime throwing anIllegalAccessError.

For example given one has a bundle A that for example contains the class foo.Bar

package foo;

public class Bar {
	void aPackagePrivateMethod() {
	}
}

and a bundle B that depends on A and has a class foo.Car

package foo;

public class Car {
	public void start() {
		new Bar().aPackagePrivateMethod();
	}
}

then there is no compiler error but during execution the an IllegalAccessError is thrown.

If B were a fragment of A, then things would be different since both would share the same class-loader. But if only B depends on A, then Bar and Car are not in the same package even if their packages have the same name.

My first guess is that this is because for a single plugin project the compile classpath is flat and all dependencies are just added to it by the Plug-in Dependencies container.

@gireeshpunathil
Copy link
Contributor

how does Bar resolve (for the compiler) in the first place? with B consuming A, should an import foo; be needed in Car to source in Bar? and that can thoroughly confuse the compiler? for example we are already in package foo, and import foo does not make sense?

@HannesWell
Copy link
Member Author

I can add import foo.Bar; and the compiler just adds a warning that it is not used.
So it really looks like the compiler thinks that both classes are in the same package.

Does anybody know if there is a way (and if yes how) to tell the compiler that although the packages have the same name they are not the same?

@iloveeclipse
Copy link
Member

Does anybody know if there is a way (and if yes how) to tell the compiler that although the packages have the same name they are not the same?

Compiler doesn't know that the code will run in OSGI context in two different bundles. The code could be compiled and put at the end in a single jar - compiler doesn't care about that as of today.

@HannesWell
Copy link
Member Author

Does anybody know if there is a way (and if yes how) to tell the compiler that although the packages have the same name they are not the same?

Compiler doesn't know that the code will run in OSGI context in two different bundles. The code could be compiled and put at the end in a single jar - compiler doesn't care about that as of today.

Yes. That confirms my assumption.
Do you think it is feasible that this can be changed/configured one day and it's worth to create a JDT issue for that?

There are already ways to express that some packages are not exported and therefore not visible. IIRC this is done through some filters or access-rules? But they are probably not so fine grained as required to solve this, are they?

I wonder if the other bundle could somehow be represented like another Java-module and restrict access via that way? But AFAIK Java modules don't permit split-packages so it is questionable if that could be a solution.

@iloveeclipse
Copy link
Member

@stephan-herrmann is a big fan of module system AFAIK (sarcasm) so may be he can give some insights if any extra configuration with access rules etc could be interpreted by compiler to issue an extra warning like "trying to use package private method in a split package". This could be configured to be an error per project, so it wouldn't be an issue in fragments.

@merks
Copy link
Contributor

merks commented Apr 17, 2024

Probably there are other fish to fry that will benefit more people more. Just saying…

@stephan-herrmann
Copy link
Contributor

@stephan-herrmann is a big fan of module system AFAIK (sarcasm)

fan or sarcasm depends on whether or not the module system is done well :)

And guess what, ecj is able to respond in this situation by saying one or both of:

  • Car.java:5 Bar cannot be resolved to a type
  • Car.java:1 The package foo conflicts with a package accessible from another module: A

But that's not in the OSGi module system, that's using JPMS, ups.

Before Java 9, ecj & pde was ahead of the bunch by being able to respect exports and dependencies at the bundle and package levels. But ecj still operated under the assumption that a package exists only once. Once you place a compilation unit in a package, ecj has no additional information about its origin.

Come JPMS and we were forced to get rid of that assumption. In fact since those days ecj internally has an explicit concept of split packages, tracking the origin of each slice of that split. But how to associate different compilation units to different packages of the same name? The compiler had to learn reading module descriptors, associating modules to source folders etc. All this was done for JPMS but not (yet) for OSGi.

Now, if JPMS would have paid respect to OSGi, both would be sufficiently similar so the compiler could see both as instances of a common concept of modules.

Without such generalization, it might still be possible to re-implement portions of PDE as to provide the information about bundles in a JPMS-like fashion. Then also packages split between OSGi bundles could be handled by ecj as split packages and you'd get all kinds of interesting new errors. Some might be stricter than you like, YMMV.

Surprisingly, one of the more interesting errors is not specified by JPMS, but compilers can still warn you, see bug 520826. As this kind of problems actually falls into the domain of API tooling, things get even more confusing as now we have 3 tools and 2 module systems treading the same ground.

I don't see any free lunch here.

<sarcasm>Maybe we should create a new module system. </sarcasm>

@stephan-herrmann
Copy link
Contributor

Closer to the point:

There are already ways to express that some packages are not exported and therefore not visible. IIRC this is done through some filters or access-rules? But they are probably not so fine grained as required to solve this, are they?

Right, access rules are name patterns. How would a name pattern express "forbid access to all package private members in foo/Bar/*"? Well maybe they actually could. But for this ecj would need to be informed not only about negative access rules, but the positive access rule ("this type is exported") would need to be passed along. This would come with some overhead, I guess. Would it help to detect this particular problem (private member from "foreign" type)? Might work. Does it generally help to better understand OSGi modules? I doubt it.

Let me add the anecdote of yet another access level: "forb-hidden". At some point this was proposed to express: "this type is visible to my dependencies but not to me". That would have resolved the dreaded error regarding indirectly referenced classes that could not be found. Perhaps the fact that this idea never took off indicates that the masters at that time were not convinced that putting more information into access rules is a good direction? I don't know.

Since we have JPMS, investing more in access rules feels a bit anachronistic, if you ask me.

@laeubi
Copy link
Contributor

laeubi commented Apr 18, 2024

In the end this shows that split-packages are just bad to the bone ...

@stephan-herrmann I think the information should somehow be already there as we have the situation that one package is coming from "the project" and one "from the classpath" and the later has an access rule (e.g. allows access to the package X). But it all sounds like a very very special situation that generally should be avoided (e.g. no split packages in the first place) therefore I tend to agree with @merks that adding tooling support for this looks not something that will pay back the complications and efforts.

@HannesWell
Copy link
Member Author

In the end this shows that split-packages are just bad to the bone ...

That's right and I didn't say that code was perfect. It just took me some time to figure out why it compiles but fails in that usual way at runtime, since I wasn't aware of the split package in that moment. Now it seems clear but I created this issue because I thought it would be good to help others in the same situation, if there were a relatively simple solution for that.
Or at least raise awareness about this nasty problem.

I don't see any free lunch here.

Since we have JPMS, investing more in access rules feels a bit anachronistic, if you ask me.

Thank you @stephan-herrmann for your elaboration. With that I also think that its currently not worth the time (at least for me).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants