[packagekit] Canceling background transaction

[Richard Hughes]

On 15 March 2011 07:41, Zhang, Qiang Z <qiang.z.zhang at intel.com> wrote:
> This ideal need support of backend, so if backend does not support cancel, this still can't work well.

Agree. I think cancel should become more important as we get cleverer
in scheduling things at the right time. For instance, in gnome-shell,
we'll want to cancel any update download before suspending, rather
than just telling the user they can't suspend.

> AFAIK, zypp backend can't support cancel feature, and zypp is wrote in c++ and launch a thread for most of operations, so it's not very easy to implement cancel.

Ohh, it's trivial to support cancellation. It's normally just a case
of fixing the library to do the right thing. I wrote zif in C, and
that supports cancellation using GCancellable, but that's because it's
written for GLib. It's still really easy to do this in C++, and
perhaps easier as you get to use exceptions.

> I have reviewed the implementation of aptcc, and have wrote a raw patch for zypp to implement cancel for download/install packages. In this patch, I put core code in an process, which is forked in a thread, and I remember the sub process ID, and once user want to cancel a transaction, we can simply call ' kill (child_pid, SIGINT)' to kill the sub process.

I'm not sure that's the right way to do it, see below.

> Any other good way to implement cancel operation for backends?

What a good library that's designed to be multithreaded (or run in a
thread) should do is have a single bit of memory available to all
functions that is simply a "is cancelled" flag.

So for instance, If I could write this for zypp, I would add a
per-thread data member of:

uint    is_cancelled;

And then after every blocking operation is completed, check this flag
to see if the transaction should be cancelled, e.g.

// download packages
emit(allow_cancel, TRUE);
curl_download (state->package_id)

// check cancelled
if (state->is_cancelled)
  throw Cancelled;

// install packages
emit(allow_cancel, FALSE);
exec('rpm -Uvh %s', % package_id)
emit(allow_cancel, TRUE);

// check cancelled
if (state->is_cancelled)
  throw Cancelled;

// refresh database
emit(allow_cancel, FALSE);
exec('updatedb')
emit(allow_cancel, TRUE);

// check cancelled
if (state->is_cancelled)
  throw Cancelled;

If you want to be clever, you also check state->is_cancelled when the
packages are downloading, so in the zif backend I'm checking the
cancelled flag every percentage change and aborting the download if
it's set. If you want to be super clever, making is_cancelled a proper
object (like a GCancellable) you can get a signal (or a vfunc, or
whatever) when the thread is signaled to cancel, and this can kill()
the updatedb type process.

Basically, I know it's a lot of boring repetitive work, but
multithreaded libraries really need to put in the work to be properly
cancelable. And testing one byte (really, one bit) of cached memory a
few thousand times in a 3 second operation doesn't even appear on the
profiling data. You might be able to do all the polling in the
pk-backend-zypp.c file, but that's architecturally at the wrong level.

Using pthread_kill() is such a horrible hack that nobody should be
using, it's like firing a rocket propelled grenade into a rally car
just to get it to pull into the pits.

Also, from personal experience, using signals to talk to processes is
really hard to do correctly, especially when rpm is involved.

Richard.