Bug 40192 - build cancel during weaving
Summary: build cancel during weaving
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: IDE (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P2 enhancement (vote)
Target Milestone: 1.2   Edit
Assignee: Andrew Clement CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-07-16 06:57 EDT by Mik Kersten CLA
Modified: 2004-03-19 10:14 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mik Kersten CLA 2003-07-16 06:57:36 EDT
The build can not be cancelled during the bytecode weaving stage.
Comment 1 Andrew Clement CLA 2004-02-25 08:20:30 EST
Two options that I can see:

Extend the IProgressListener interface
  boolean isCanceled()
Which we then check in the BcelWeaver

Extend the IMessageHandler interface
  boolean cancelRequested()
Which we can check in multiple places - probably BcelWeaver and below 
(BcelClassWeaver).

---
The first case makes sense if we consider the only cancellation use case to be 
via some kind of UI/IDE that is monitoring the compilation and may want to 
stop it.

The second case makes sense if we think cancellation is a more general kind of 
request that any compiler/weaver user might request.
---
any comments Mik?
Comment 2 Mik Kersten CLA 2004-02-25 11:55:46 EST
I just spent some time on this since in AJDE we still have a bunch of the 
infrastructure that made canceling work properly in 1.0.x.  So I'll need to 
clean that up, since the cancelling functionality already exists in other 
places (and AJBrowser, JBuidler, and NetBeans are coupled to it).

Wh don't you go ahead with your first option and extend IProgressListener, 
probably with setCancelledRequested(boolean) and isCancelledRequested() 
methods.  I believe that this is actually general enough, and that any client 
capable of cancelling builds/weaves should be aware of that progress 
listener.  The IMessageHandler is not appropriate for this.  Note that you'll 
need to add an implementation to AJDE's BuildNotifierAdapter -- just make that 
a stub since that's part of what I need to clean up.

Please update the report you've added the checks and have a test case in AJDE 
that checks the cancel during both compile and weave.  I wonder if the test 
case needs to check that there are no partly-written classfiles on disk.  But 
proper error/abort handling in the weaver should have ensured of that already.

After that I'll update AJDE accordingly, clean out the old canceling logic 
from the buld and compile manager and make sure it works in the Swing 
clients.  After that making it work for AJDT will be trivial.
Comment 3 Andrew Clement CLA 2004-03-04 06:31:25 EST
As Mik suggests, I have extended IProgressListener.  I then picked some key 
points that seemed sensible valid states in which to check for cancellation:

1) Between file compilations
2) Between file weaves

In either of these states we are 'consistent' and can exit cleanly knowing 
what is on the disk and we are not half way through doing anything.
Aborting half way through weaving a file or half way through compiling an 
individual file is risky.

So, for each case:

1) Between file compilations

AjBuildManager.acceptResult() is called.  At the end of acceptResult() check 
if cancel is requested, throw 
   new AbortCompilation(true,new CompilationCancelledException());

AbortCompilation is a valid mechanism for the JDT compiler to exit, true means 
silently, the CompilationCancelledException is a new AJ type that will be 
thrown from the AbortCompilation handling code that will percolate up to 
performCompilation().  We simply put a catch around the compiler.compile() 
call in the performCompilation method and just return.

2) Between file weaves
The least intrusive change here is to check for cancellation at the start of 
the BcelWeaver.weave(UnwovenClassFile,BcelObjectType,boolean) method and have 
a WeavingCancelledException thrown which is then caught at the top level weave
() method that has a try...catch around all of its activities.  The weave() 
method is defined to return a Collection of files that were successfully 
woven - and putting the try..catch and exception throwing logic at the places 
suggested ensures weave() still returns correctly with the list of files it 
managed to weave successfully. (Just in case the code calling weave() makes 
some assumption about the results it gets back).  Of course, any caller of the 
compiler/weaver that used the cancellation mechanism will know that it cannot 
rely on the results.

Anyway, all the changes above total about 30lines of code so they don't get in 
the way of understanding what the compiler/weaver are doing today.  But I'm 
not checking this feature in right now - principally because Adrian is 
changing the whole compile/weave flow and that will impact this design.
Comment 4 Jim Hugunin CLA 2004-03-04 16:44:52 EST
I think your points to interrupt compilation sound right.  I'd implement it a 
little bit differently.

For the weaver I'd modify the BcelWeaver.weave() method to insert this check 
in its various loops.  This feels much cleaner to me than throwing an 
exception.

For the compiler I think you want to understand how the JDT interrupts 
compilations.  You'll need to be compatible with this anyway for the IDE 
work.  Whatever mechanism is used there is probably the right one to use to 
interrupt ajdt compilation.
Comment 5 Andrew Clement CLA 2004-03-18 08:21:34 EST
This was more straightforward after Adrians changes to the compilation/weaving 
process.  Now there is a single catch block around compiler.compile() that 
catchs OperationCancelledException.  When we wish to abort either compilation 
or weaving, we throw an AbortException containing an 
OperationCancelledException - this is the JDT way of doing things.  JDT 
unwraps it and the OperationCancelledException percolates out to where we can 
see it and catch it.  There are two points where we check for cancellation:
- After compiling a file
- After weaving a file
BuildCancellingTest has been added to ajde tests to verify the behavior is as 
expected.

In the process of getting the progresslistener into the right place in the 
weaver for checking cancellation I added some suitable progress feedback 
(percentages and messages) for weaving.
Comment 6 Andrew Clement CLA 2004-03-19 10:14:29 EST
Changes now building successfully on our build machine.