Comment 1 Stephan Herrmann 2020-12-06 14:16:15 EST

I might add that the exception was observed with Object Teams in the loop (i.e., a variant of jdt.core, but updated to latest as of jdt's RC2) - hence the chance also for line number mismatches.

I hold, however, that the above analysis clearly shows a bug in JDT.

Comment 2 Stephan Herrmann 2020-12-06 16:39:24 EST

I tried to reproduce, but the bug is more elusive than I thought, so perhaps not so many users affected as I feared.

Comment 3 Eclipse Genie 2020-12-06 17:25:49 EST

New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/173479

Comment 4 Stephan Herrmann 2020-12-06 17:36:00 EST

(In reply to Eclipse Genie from comment #3)
> New Gerrit change created:
> https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/173479

Regression test succeeded to piggy back on an existing test from Bug 224708.

Error occurs in the context of Project2 depending on modular Project1 and both having test folders & test dependencies. Not all of that may be needed for reproducing, but without a fix this test succeeds to demonstrate the exception (on the console & a not-ok status).

Fixed from both sides:
* ignore not-castable updates
* avoid lambda where possible to allow casting

This still leaves out persisting of two more updates, those representing MODULE_MAIN_CLASS and MODULE_PACKAGES. So one may want to revisit the story to decide if those should go into the state, too. But currently those would be elegantly ignored.

@Gunnar, do you want to assess severity and organize one of: 
((roll back bug 567532 or get approval for this fix here) & get approval for a re-spin), 
OR 
(argue why this is not a severe regression)?

Comment 5 Gunnar Wagenknecht 2020-12-07 06:11:14 EST

(In reply to Stephan Herrmann from comment #4)
> (In reply to Eclipse Genie from comment #3)
> > New Gerrit change created:
> > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/173479
> 
> Regression test succeeded to piggy back on an existing test from Bug 224708.

I checked out the Gerrit. There is no change in ModuleBuilderTest other than an import. Did you add a new test that reproduces the issue?

> Error occurs in the context of Project2 depending on modular Project1 and
> both having test folders & test dependencies. Not all of that may be needed
> for reproducing, but without a fix this test succeeds to demonstrate the
> exception (on the console & a not-ok status).

Should this test fail now? I ran them locally without your changes and they pass. Thus, I'm confused about what should fail and when.

> @Gunnar, do you want to assess severity and organize one of: 
> ((roll back bug 567532 or get approval for this fix here) & get approval for
> a re-spin), 
> OR 
> (argue why this is not a severe regression)?

I'm still unable to re-produce. Thus, I'm wondering if this is something more specific and not affecting a lot range of users.

I think merging your change makes sense nonetheless.

Comment 6 Stephan Herrmann 2020-12-07 08:02:56 EST

(In reply to Gunnar Wagenknecht from comment #5)
> (In reply to Stephan Herrmann from comment #4)
> > (In reply to Eclipse Genie from comment #3)
> > > New Gerrit change created:
> > > https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/173479
> > 
> > Regression test succeeded to piggy back on an existing test from Bug 224708.
> 
> I checked out the Gerrit. There is no change in ModuleBuilderTest other than
> an import. Did you add a new test that reproduces the issue?

I added a paragraph to the existing test testWithTestAttributeAndTestDependencyOnClassPath().

You need to save the workspace to trigger the exception, you need to dig into the returned IStatus to detect that there was an exception.
 
> > Error occurs in the context of Project2 depending on modular Project1 and
> > both having test folders & test dependencies. Not all of that may be needed
> > for reproducing, but without a fix this test succeeds to demonstrate the
> > exception (on the console & a not-ok status).
> 
> Should this test fail now? I ran them locally without your changes and they
> pass. Thus, I'm confused about what should fail and when.

Yes, I saw the modified testWithTestAttributeAndTestDependencyOnClassPath() failing without the other changes.

Comment 7 Eclipse Genie 2021-01-03 11:42:20 EST

New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/174193

Comment 8 Andrey Loskutov 2021-01-04 08:07:31 EST

*** Bug 570048 has been marked as a duplicate of this bug. ***

Comment 9 Stephan Herrmann 2021-01-04 11:57:20 EST

In gerrit Gunnar asked me, what I expected from him. To make my response visible outside gerrit, I'm giving a general answer here in the form of an ordered wish list:

1. commits should not introduce regressions :)

since 100% of (1) is impossible go to (2)

2. it would be nice if the author of a regression-causing commit would take full responsibility for fixing, so no (other) committer needs to invest time.

since the actual mistake in this case was outside the latest commit, go to (3)

3. if the actual mistake is in code contributed by s.o. else, if would be nice if that person was contacted so s/he can fix their part.

since that other person was Till, who no longer contributes to JDT, go to (4)

4. when a proposed fix exists, it would be nice if the author for the regression-causing commit would confirm the fix by
- confirming the bug and that a test exists demonstrating the bug
- confirm that the fix looks good and fixes the regression test

Multiple benefits of (4):
- fix & test are checked my several pairs of eye-balls.
- contributor learns about the code and can be more readily trusted when contributing to the same code area again.
- team learns that contributor is not a hit-and-run person but cares about their contribution beyond the initial commit.


So much for my general picture.

Specifically, I had noticed the bug right during the RC phase, found it quite severe, and wanted to give a chance that a fix be included in the release. So I hurried to provide a draft fix. Including the fix in the release didn't happen. Meanwhile we're in M1 freeze phase, and still the regression exists in master.

I saw that Gunnar had a look at some part of the fix, but I saw no confirmation that he understood the root cause of the regression. Hence the responsibility for fixing would be solely on my shoulders, which was not my intention (otherwise I would have assigned the bug to myself).

Alternative, of course, whoever reviewed and merged the change can adopt the entire business and drive the fix process.

The follow-up bug 570031 can be taken up individually at a later point.

Comment 10 Gunnar Wagenknecht 2021-01-04 12:52:58 EST

(In reply to Stephan Herrmann from comment #9)
> So much for my general picture.

Thanks Stephan. I think we are in agreement with regards to the general picture. I'm 100% willing to provide a fix.

> I saw that Gunnar had a look at some part of the fix, but I saw no
> confirmation that he understood the root cause of the regression. Hence the
> responsibility for fixing would be solely on my shoulders, which was not my
> intention (otherwise I would have assigned the bug to myself).

I can confirm that I do understand the technical aspects of the exception and what is causing it. I'm lacking context on how to re-produce it. It doesn't seem to affect many users, eg., only two reports so far (including yours). That probably explains why we didn't find it when working on the change back then. 

Of course, it's possible to write a test that would exploit it programmatically. I would feel more comfortable with a sample project and wrapping a test around that.

I also need guidance on what the appropriate fix should look like. It looks like there are multiple possibilities to tackle this one.

Comment 11 LHEZ Gaël 2021-02-06 15:55:48 EST

Hello,

I had the same exception: I am using the latest Java 2020.12 release built using Oomph and some plugins (I don't think this is related, I'll omit them for clarity):

  Eclipse Java Development Tools	3.18.600.v20201202-1800	org.eclipse.jdt.feature.group	Eclipse.org

It always appear when I save the workspace (which is the "use case" if I understand well the comment above):

!SUBENTRY 1 org.eclipse.jdt.core 4 2 2021-02-06 21:34:56.457
!MESSAGE Error saving last build state for project jtools-token-grep
!STACK 0
java.lang.ClassCastException: class org.eclipse.jdt.internal.core.ModuleUpdater$$Lambda$1039/0x0000000801cbde80 cannot be cast to class org.eclipse.jdt.internal.compiler.env.IUpdatableModule$AddReads (org.eclipse.jdt.internal.core.ModuleUpdater$$Lambda$1039/0x0000000801cbde80 and org.eclipse.jdt.internal.compiler.env.IUpdatableModule$AddReads are in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @3f04fb07)
	at org.eclipse.jdt.internal.core.builder.State.writeBinaryLocations(State.java:811)
	at org.eclipse.jdt.internal.core.builder.State.write(State.java:519)
	at org.eclipse.jdt.internal.core.builder.JavaBuilder.writeState(JavaBuilder.java:165)
	at org.eclipse.jdt.internal.core.JavaModelManager.saveBuiltState(JavaModelManager.java:4393)
	at org.eclipse.jdt.internal.core.JavaModelManager.saveState(JavaModelManager.java:4372)
	at org.eclipse.jdt.internal.core.JavaModelManager.saving(JavaModelManager.java:4729)
	at org.eclipse.core.internal.resources.SaveManager.executeLifecycle(SaveManager.java:387)
	at org.eclipse.core.internal.resources.SaveManager$1.run(SaveManager.java:204)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45)
	at org.eclipse.core.internal.resources.SaveManager.broadcastLifecycle(SaveManager.java:207)
	at org.eclipse.core.internal.resources.SaveManager.save(SaveManager.java:1164)
	at org.eclipse.core.internal.resources.Workspace.save(Workspace.java:2333)
	at org.eclipse.ui.internal.ide.application.IDEWorkbenchAdvisor.lambda$1(IDEWorkbenchAdvisor.java:509)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:122)

If that may help, my projects are using m2e and it does not always fails:

1) You may try importing (Import as Maven project) them here: https://github.com/glhez/tools/blob/master/jtools-parent/pom.xml
2) Restart -> it may fail
3) Do a refresh of project (Alt + F5)
4) Restart -> it may fail

I've done a quick test and it failed twice with the given workflow.

Comment 12 Stephan Herrmann 2021-02-17 04:54:07 EST

this has fallen through the cracks

While JDT is in M3 stabilization I still think this *needs* to go into 4.19.

Summarizing the status from memory:

* the gerrit change modified a test so that it should deterministically trigger the bug until the fix from the same gerrit is also applied.

* the fix in gerrit should be good.

* note that filtering with an isInstance() check is *not* the fix, it's just a safety net, the fix is in avoiding the lambda that caused the CCE

This bug needs an owner who ensures that 4.19 does not sail without the fix.

Comment 13 Manoj N Palat 2021-02-19 07:40:00 EST

(In reply to Gunnar Wagenknecht from comment #10)
> (In reply to Stephan Herrmann from comment #9)
> > So much for my general picture.
> 
> Thanks Stephan. I think we are in agreement with regards to the general
> picture. I'm 100% willing to provide a fix.
> 

@Gunnar, would you be able to provide a fix for this in RC1 timeframe? [moving the target to 4.19RC1 - that's just early next week.

Comment 14 Gunnar Wagenknecht 2021-02-22 03:08:32 EST

(In reply to Manoj Palat from comment #13)
> @Gunnar, would you be able to provide a fix for this in RC1 timeframe?
> [moving the target to 4.19RC1 - that's just early next week.

Confirmed! I'm updating the review towards master today.

Comment 15 Jay Arthanareeswaran 2021-02-23 00:25:23 EST

+1 for RC1.

Comment 16 Eclipse Genie 2021-02-23 05:14:42 EST

Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/174193 was merged to [master].
Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=37880e206254df54740ec4b0ba10835836d6ad22