Bug 265047

Comment 1 Gary Karasiuk 2010-05-26 15:59:10 EDT

Created attachment 170090 [details]
delay the shutdown patch

Comment 2 Wini Mark 2010-05-27 10:32:40 EDT

Thanks Gary. I have tested with the patch with deleting the resource during the validation, and it works and there is no errors in the .log. I also run some regression test (Build validation and manual validation on some projects, e.g. EE and web project).

Comment 3 Carl Anderson 2010-05-27 10:39:20 EDT

I approve of this patch.

Comment 4 Carl Anderson 2010-05-27 10:46:56 EDT

This is a nasty bug due to the errors that occur when the Validation Framework stops before validation has finished.

There is no workaround.
 
This was tested by hand, as documented in comment #2

Simply put, the solution is for the Validation Framework to do a join() with the validation job, thus ensuring that the Framework does not terminate until the Validation is finished.  Wini Mark and myself have reviewed Gary's patch.

This is a low risk fix, limited only to the Validation Framework.

Comment 5 Tim deBoer 2010-05-27 11:04:17 EDT

What happens if you open a large workspace and then decide to close it right away after validation has kicked off? Does it just block for the currently running validator, or do you have to wait (potentially a very long time) for the full workspace to finish validation?

Comment 6 Gary Karasiuk 2010-05-27 12:01:53 EDT

(In reply to comment #5)
I believe that the behavior would now be similar to what happens if you were in the middle of a build ... the workbench would not shutdown until the operation finished.

I haven't tried this, but I think you would still have a progress monitor running, and you could cancel the validation job, to speed up the shutdown.

Comment 7 Gary Karasiuk 2010-05-27 12:06:21 EDT

(In reply to comment #6)
I think it is too late in the cycle to do anything fancier than this, but in the next release, I think there should be way to "hibernate" validation, so that the shutdown is not delayed.

Comment 8 Tim deBoer 2010-05-27 14:13:13 EDT

Sorry to prolong this, but can someone confirm that cancelling the job has reasonable behaviour? (i.e. perhaps a single validator won't cancel immediately, but when moving to a new validator or new project the framework does cancel appropriately and exit) Just want to make sure we're not causing the equivalent of a 'hang' on shutdown.

Comment 9 Tim deBoer 2010-05-27 14:17:17 EDT

Based on Valentin's comments in the weekly WTP call, the individual validators should respond to cancelation appropriately, and if not the framework will check between validators. +1

Comment 10 Gary Karasiuk 2010-05-27 14:22:18 EDT

(In reply to comment #8)
> Sorry to prolong this, but can someone confirm that cancelling the job has
> reasonable behaviour? (i.e. perhaps a single validator won't cancel
> immediately, but when moving to a new validator or new project the framework
> does cancel appropriately and exit) Just want to make sure we're not causing
> the equivalent of a 'hang' on shutdown.
Wini can you test this?

I'm 95% certain that this will behave as you suggest. All that the patch is doing is joining the job, so if the job is now gone, the shutdown method should exit.

Comment 11 Valentin Baciu 2010-05-27 14:25:04 EDT

In org.eclipse.wst.validation.internal.ValManager.accept(IValidatorVisitor, IProject, ValType, ValOperation, IProgressMonitor), the code checks for the progress monitor isCanceled() and returns.

	public void accept(IValidatorVisitor visitor, IProject project, ValType valType, 
		ValOperation operation, IProgressMonitor monitor){
		
		if (isDisabled(project))return;
		
		for (Validator val : getValidators(project)){
			if (monitor.isCanceled())return;
.....

Comment 12 Wini Mark 2010-05-27 14:39:34 EDT

(In reply to comment #10)
> (In reply to comment #8)
> > Sorry to prolong this, but can someone confirm that cancelling the job has
> > reasonable behaviour? (i.e. perhaps a single validator won't cancel
> > immediately, but when moving to a new validator or new project the framework
> > does cancel appropriately and exit) Just want to make sure we're not causing
> > the equivalent of a 'hang' on shutdown.
> Wini can you test this?
> 
> I'm 95% certain that this will behave as you suggest. All that the patch is
> doing is joining the job, so if the job is now gone, the shutdown method should
> exit.

I run a simple test that I click the 'cancel' in the progress dialog and the validation job is canceled and the workspace is shut down pretty quickly and I don't see the 'hang'.

Comment 13 Carl Anderson 2010-05-27 14:48:32 EDT

Committed to HEAD for WTP 3.2 RC3

Comment 14 David Williams 2010-05-27 15:40:59 EDT

Wini, you accidentally reset the pmc approved flags to '?'. 

Its a bugzilla/webpage problem. Even if you hit 'refresh' sometimes, its doesn't correctly pick up flag changes, and doesn't show them as "midair collision". I've found it helps to actually click on the bug number at top of bug report to make sure it loads completely fresh fields. 

But, its all in history if there's ever any doubt. 

Thanks again for fixing this!

Comment 15 Wini Mark 2010-05-27 15:45:36 EDT

David, sorry for causing the confusion. I will remember not to do the refresh and click on the bug number in the future :)  Thanks.