Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 330190

Summary:	Wrong synchronization in jdt.internal.compiler.Compiler
Product:	[Eclipse Project] JDT	Reporter:	daolaf
Component:	Core	Assignee:	JDT-Core-Inbox <jdt-core-inbox>
Status:	CLOSED WONTFIX	QA Contact:
Severity:	normal
Priority:	P3	CC:	Olivier_Thomann, stephan.herrmann
Version:	3.7
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Whiteboard:	stalebug

Description daolaf

2010-11-13 20:37:22 EST

Build Identifier: 

There is some inconsistent synchronization done in the jdt.inernal.compiler.Compiler class.

The array unitsToProcess is guarded at several places - mostly together with the int totalUnits - in a synchronized block. However, in some methods it is not guarded at all (e.g. processAnnotations or compile). If the code is really supposed to be called by multiple threads in parallel then this might lead to ArrayIndexOutOfBoundsExceptions, NullpointerExceptions, etc.

I don't know the code well enough to provide a patch, but someone should look at this. Either the code is supposed to be called by multiple threads in parallel, then the guarding of unitsToProcess and totalUnits must be improved, or it is not going to be called in parallel, then the synchronization can be completely removed.

Reproducible: Always

Comment 1 Stephan Herrmann

2010-11-14 07:48:00 EST

I'm not sure whether or not synchronization is inconsistent, but it might
indeed be a good idea to make the contracts a bit more explicit.

While I'm trying to understand this aspect of the code I see these:

Currently, concurrency only happens in one specific block within compile(),
starting with
  processingTask = new ProcessTaskManager(this);
and ranging up to
  if (processingTask != null) {
	processingTask.shutdown();
	processingTask = null;
  }

So, e.g., the three uses of this.unitsToProcess just above that block
are explicitly guarded by "if (this.useSingleThread)".

More specifically, each instance of Compiler is only accessed from two
different threads: the one calling compile and the one spawned for the
ProcessTaskManager. The interface between these two objects is quite
narrow and perhaps it would be clearer if ProcessTaskManager would access
the compiler not via its class but via an interface that exposes only a
few thread-safe methods. This would show that ProcessTaskManager cannot
invoke any of the methods that unguardedly access Compiler.unitsToProcess.
Note, however, that the three accept(..) methods are in fact part of the
protocol (invoked indirectly via LookupEnvironment.askForType(..)).
It should be possible to show (although far from trivial) that concurrent
access to Compiler.unitsToProcess indeed only happens through the two
synchronized methods addCompilationUnit(..) and getUnitToProcess(..).

Of course things could change dramatically once we allow more concurrency
like bug 126121 and bug 151053 requested. But these indicate much harder
problems needing to be addressed: synchronize workspace access (builder)
and binding cache (compiler).

not a definite answer, just documenting my current understanding.

Comment 2 Eclipse Genie

2019-11-30 06:00:10 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.