eclipse-jdt / eclipse.jdt.debug

Eclipse Public License 2.0
16 stars 46 forks source link

DetectVMInstallationsJob: defer detection until needed. #293

Closed jukzi closed 1 year ago

jukzi commented 1 year ago

So that it does not run in jdt.core tests at all

https://github.com/eclipse-jdt/eclipse.jdt.core/issues/1190

jukzi commented 1 year ago

for me it fixes locally running org.eclipse.jdt.core.tests.model.JavaSearchBugsTests.testBug178847()

jukzi commented 1 year ago

@mickaelistria please review if that still fulfills the original intend of DetectVMInstallationsJob

jjohnstn commented 1 year ago

@jukzi FYI @mickaelistria is on vacation until after Tuesday.

jukzi commented 1 year ago

@jjohnstn can you review? You did participate in the initial commit.

jukzi commented 1 year ago

I tested the following: If i have an empty workspace, the jdk detection runs as soon right before i open JDK preference dialog. If i have a workspace containing a java project the detection runs while eclipse is starting during "Initializing Java Tooling 0%"

Thread [Worker-3: Initializing Java Tooling] (Suspended (breakpoint at line 51 in DetectVMInstallationsJob))    
    owns: Object  (id=74)   
    DetectVMInstallationsJob.<init>() line: 51  
    DetectVMInstallationsJob.initialize() line: 194 
    JavaRuntime.initializeVMs() line: 3273  
    JavaRuntime.getVMInstallTypes() line: 574   
    EnvironmentsManager.initializeCompatibilities() line: 309   
    ExecutionEnvironment.init() line: 185   
    ExecutionEnvironment.getDefaultVM() line: 227   
    JREContainerInitializer.resolveVM(IExecutionEnvironment) line: 179  
    JREContainerInitializer.resolveVM(IPath) line: 146  
    JREContainerInitializer.initialize(IPath, IJavaProject) line: 63    
    JavaModelManager.initializeContainer(IJavaProject, IPath) line: 3209    
    JavaModelManager$10.run(IProgressMonitor) line: 3093    
    Workspace.run(ICoreRunnable, ISchedulingRule, int, IProgressMonitor) line: 2453 
    Workspace.run(IWorkspaceRunnable, ISchedulingRule, int, IProgressMonitor) line: 2478    
    JavaModelManager.initializeAllContainers(IJavaProject, IPath) line: 3155    
    JavaModelManager.getClasspathContainer(IPath, IJavaProject) line: 2152  
    JavaCore.initializeAfterLoad(IProgressMonitor) line: 4610   
    InitializeAfterLoadJob$RealJob.run(IProgressMonitor) line: 39   
    Worker.run() line: 63   
jjohnstn commented 1 year ago

@jukzi I'll give it a look. Any idea about the test failures? Do you need to rebase?

jukzi commented 1 year ago

Any idea about the test failures?

those are only random. i just rebased and had no errors before. See https://ci.eclipse.org/jdt/job/eclipse.jdt.debug-github/job/PR-293/1/

jjohnstn commented 1 year ago

Hi @jukzi I tried running the tests locally and they run fine. I ran into a problem until I switched from using the JustJ JRE to a Java 17 JDK I had. Is is possible that the JDI tests are finding a bogus VM that has been created by other tests and then it is being discovered? I am fine with the patch and will approve, but you might want to try disabling the detection when running the JDI tests to see if that fixes the problem. I have to leave and won't be back in time for the deadline.

jukzi commented 1 year ago

I switched from using the JustJ JRE to a Java 17 JDK I had

That sounds like a local error only - which we could fix in future.

mickaelistria commented 1 year ago

Thanks!