Re: [cdt-dev] [DSF] Making Sequence more robust

[Pawel Piech <pawel.piech@xxxxxxxxxxxxx>]

I think there's at least a couple things we could do to improve this:

Report runtime exceptions thrown in DSF executor.
Currently a quirk in the implementation of ScheduledThreadPoolExecutor keeps us from catching these exceptions unless assertions are enabled.  If a brave souls steps up to do a clean-room reimplement this class we could get around this bug.  It wouldn't need to be as efficient or as complicated implementation because we only need a single thread.  Or maybe someone more clever can think of a way to get around this problem without reimplementing or degrading performance. (bug 236915)

Create a registry for outstanding request monitors.
Most request monitors are passed around between functions, sometimes directly as parameters to functions, and sometimes as parents to other RMs.  There's only a few clients outside of the DSF framework that collect request monitors and don't complete them immediately: caches and things like the GDB command control.  We could catch the lost RMs if we create a registry where RMs are added when created and removed when collected by a service or another component.  So a few example flows would be:

Simple completion
- An RM is created and added to the registry. 
- A service calls RM.done() and the RM is removed from the registry.

Waiting on async completion
- An RM is created and added to the registry. 
- A service sends a command to an asynchronous data provider (command control)
- The service manually removes the RM from registry.

RM cached, waiting on another RM to complete
- An RM is created and added to the registry. 
- A service finds that another RM is already waiting in cache for the same result
- Cache removes the RM from registry.

Stacked RM waiting on async completion
- An RM is created and added to the registry. 
- Another method takes the RM and adds it as a parent to another RM.
- A service sends a command to async. data provider:
- Service removes RM from registry. 
- RM removes its parent from registry too.

At the end of any sequence there should be no RMs in the registry.  So a post-executor if a DSF executor could check for RMs in the registry and log exception or even immediately halt execution.

We'd chance a lot of false positives at first and we may have some hard to resolve edge cases, but with some work we'd  end up with a more predictable system.  Also, for backward compatibility, this tracking would need to be optional.

Cheers,
Pawel

On 10/16/2011 03:07 AM, Vladimir Prus wrote:

Hi,

presently, if any step of a Sequence fails to call 'done' or 'cancel',
the whole Sequence just hangs, and it's rather painful to debug.
Can we do something to improve this long-standing problem? For example:

    abstract public static class Step {

        ....

        public final void execute(RequestMonitor rm) { // note: final.
			xexecute(rm);
			if (!rm.isDone()) // isDone would have to be added
			{
				rm.setStatus(new Status(IStatus.ERROR, ..., "Misbehaving step", ....));
	            rm.done();
			}
        }

		abstract protected void xececute(RequestMonitor rm);
    }

Or, we can take take this even further:

    abstract public static class Step {

        ....

        public final void execute(RequestMonitor rm) { // note: final.
			rm.setStatis(xexecute(rm))
			rm.done();
        }

		abstract protected IStatus xececute(RequestMonitor rm);
    }

In this case, the compiler will object if a step fails to return a value,
although there's a risk that a step might fail to handle failure of any
operation it does and still return IStatus.OK anyway.

Comments?