Community
Participate
Working Groups
Build Identifier: 20090920-1017 After running a Job that has user set to true, it shows up twice in the progress view (sometimes with two different finished times). Reproducible: Always Steps to Reproduce: 1.Create a Job 2.call Job.setUser(true) 3.call Job.schedule()
Created attachment 159877 [details] test case to reproduce the problem Create a new run config. Run product "org.eclipse.platform.ide", including the plugin progressBug. Show the progress view. Click samplemenu-> same command.
I forgot to mention, one also needs to call: Job.setProperty(IProgressConstants.KEEP_PROPERTY, Boolean.TRUE);
Sounds wagely like Bug# 197258, but not a dup of that. However, if we have a Thread.sleep() in the run method the job shows only once. No time to include this in 3.6
John, looking at DetailedProgressViewer.doFindItem(Object) it seems that the equality test is wrong: shouldn't it compare the jobs instead of the JobInfo objects which change for each notification? That would explain the strangeness we sometimes seen in the ProgressView. If so, same issue in DetailedProgressViewer.add(Object[]) where a set is used to ensure no duplication which of course fails if the JobInfo is used instead of the job itself.
(In reply to comment #4) > John, looking at DetailedProgressViewer.doFindItem(Object) it seems that the > equality test is wrong: shouldn't it compare the jobs instead of the JobInfo > objects which change for each notification? I don't know this code very well, but it seems to me the intend is for there to only ever be one JobInfo for a given job for as long as that job is appearing in the progress view. ProgressManager.jobs keeps a map of Job->JobInfo to maintain this. So, comparing the JobInfo should be sufficient I think. However, looking at the two places where JobInfo objects are created, there isn't any synchronization which can clearly cause multiple JobInfo instances to exist for a job: 1) ProgressManager.getJobInfo(): JobInfo info = internalGetJobInfo(job); if (info == null) { info = new JobInfo(job); jobs.put(job, info); } If this method is called concurrently in multiple threads then multiple JobInfo could be created, because there is no synchronization between getJobInfo and creating the new one. This method is called for job event callbacks and progress callbacks, so it can easily happen in two threads concurrently. 2) ProgressManager.updateFor: if (jobs.containsKey(event.getJob())) { refreshJobInfo(getJobInfo(event.getJob())); } else { addJobInfo(new JobInfo(event.getJob())); } Again if this method is invoked concurrently, or concurrently with getJobInfo, then multiple JobInfo instances could easily be created. Both of these methods should probably do a synchronized(jobs) {... } to eliminate the timing window that could cause duplicate JobInfos for the same job (which as Dani notes will cause errors in the progress viewer).
I think we're seeing this as well, but it manifests as a memory problem for us because the Job never gets cleaned up. In the particular case I saw recently, one of our Jobs that generates a large amount of intermediate data never got GCed because the JobInfo remained in some of the JobManager structures. For now we're having to handle this defensively by explicitly clearing references at the end of run(), but obviously that leaves a memory leak, just less severe.
*** Bug 366347 has been marked as a duplicate of this bug. ***
Works in I20190901-1800.
