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

Bug 356715

Summary:	[builder] QueuedBuildData#queueURI(URI) should check if URI already is queued
Product:	[Modeling] TMF	Reporter:	Knut Wannheden <knut.wannheden>
Component:	Xtext Backlog	Assignee:	Project Inbox <tmf.xtext-inbox>
Status:	CLOSED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	lieven.lemiengre, roman.mitin, sebastian.zarnekow, sven.efftinge
Version:	2.1.0
Target Milestone:	---
Hardware:	PC
OS:	Mac OS X - Carbon (unsup.)
Whiteboard:	v2.8

Description Knut Wannheden

2011-09-05 05:31:15 EDT

Inernally the QueuedBuildData uses a Map<String, LinkedList<URI>> to store individual queues for all projects. When queueURI(URI) is called the URI is added to the list of the respective project (as determined by the IStorage2UriMapper). But the project queue is a simple LinkedList and it is not checked whether the queue already contains the given URI. This can cause the builder to build a resource twice.

This can for instance happen if the build is cancelled during the second build phase. By then the URIs will already have been queued (part of first build phase). The next build will queue the same URIs again.

This problem could also arise with an unusual IStorage2UriMapper implementation or an unusual project where URIs are mapped multiple times into the project using linked folders.

Comment 1 Knut Wannheden

2011-09-06 10:53:24 EDT

Thinking about it I think it is wrong that the builder queues URIs which later don't get removed if the build gets canceled. This means that the next build will build these resources again.

Use case: The user makes a change to a source and hits save. As a result the builder queues and starts building many resources because he made a "real" change. The user now realizes that he did something wrong and cancels the build. Next he reverts the change and hits save again. The builder will again start building all those resources, even though that's not required anymore. Now the user has to restart Eclipse if he doesn't want to go through with the build.

Raising importance to major. Please revert to normal in case I missed something.

Also note that this is related to the initially reported problem but not quite the same.

Comment 2 Sven Efftinge

2011-09-06 13:52:39 EDT

Not sure I understand the importance here. The worst thing that can possibly happen is that resources are built twice? I guess you are concerned about build time? 
Feel free to come up with a fix of the problem if it's that important for you.

Comment 3 Knut Wannheden

2011-09-07 04:58:31 EDT

Actually the worst case is worse: The JDT could queue some deltas and the next build invocation would then queue the corresponding affected resource URIs. If now this build (or a build of another project) is cancelled after already having dequeued one of these URIs, then the corresponding resources will never be rebuilt.

The QueuedBuildData should be transactional:
 - URIs queued during a build (except those queued in response to pending deltas!) should be removed from the queue if the build is cancelled.
 - URIs dequeued during the build should be enqueued again if the build is cancelled.

Comment 4 Sebastian Zarnekow

2011-09-13 11:59:18 EDT

Please feel free to provide a patch for the described problem.

Comment 5 Sebastian Zarnekow

2011-11-09 14:51:15 EST

Not 2.1

Comment 6 Roman Mitin

2015-07-24 03:43:27 EDT

Looks like this was fixed in Xtext 2.8.0 by https://github.com/eclipse/xtext/commit/ab5cba98cbd62018e3c8198e3f62daecb8294b3f . Close issue?

Comment 7 Sebastian Zarnekow

2015-07-24 04:12:10 EDT

Thanks for the book-keeping.

Comment 8 Eclipse Webmaster

2017-10-31 11:04:59 EDT

Requested via bug 522520.

-M.

Comment 9 Eclipse Webmaster

2017-10-31 11:16:20 EDT

Requested via bug 522520.

-M.