Re: Reverting to non-async file chooser

[Tim Janik <timj imendio com>]

On Wed, 23 Aug 2006, Federico Mena Quintero wrote:

> On Fri, 2006-08-18 at 11:55 +0200, Tim Janik wrote:
>
> > the reason we didn't have these semantics in the first place is that
> > because the file operations may be executed *asyncronously*, cancellation
> > can fail.
>
> Yes, cancellation can fail, but it turns out that you normally don't
> care about that.

well, most often you *do* care, because otherwise you either
wouldn't need to install the callback or you wouldn't need to cancel
an operation in the first place.

in any case, "normally don't care" simply isn't good enough because
in cases were you do indeed care if cancellation succeeded, you have
*no* way to be notified with the changes you suggest. and that'd be
a grave design bug, no matter how you put it.

> I'm going to change the current scheme to this:
>
> - Canceling an operation guarantees that the callback is not called.

please don't. that is just introducing breakage by design, and can
become very hard to fix in the future.

> > there are two ways to handle this properly, in a way that doesn't need
> > to intermix cancellation semantics with closure/memory maintenance.
> > if your closure references an object that the callback needs, you
> > can either forcefully keep that object alive through a reference
> > count until the closures gets freed (which can be early enough with
> > a high prio idler), or you set a flag in the closure/nullify the
> > object pointer in the closure once the object becomes invalid, so the
> > callback can tell it's not supposed to treat the reference as valid.
>
> This gets complicated very quickly, and requires a lot of support code.

it doesn't get complicated at all. you either explicitely ref/unref:
  void callback_func (void *data) { g_object_unref (data); }
  install_callback (callback_func, g_object_ref (data));
or nullify your pointer and add a check to the callback:
  if (!data->owner)
    return;

> A scheme like GnomeVFS's requires no particular intelligence from the
> caller.

well, the current scheme does keep it very simple on the caller:
the notification callback is always called and just has the information
on whether the operation was cancelled and if it succeeded or not.
if in any particular case, cancellation also means that the caller
has gone away, and you didn't bother setting up a nullify pointer
handler or ref/unref pair, you can still bail out early in your
callback with:
  if (cancelled)
    return;

i.e. effectively render your callback inoperable, without
introducing information loss by design.

> > >  Caveat:  gnome-vfs does not let you cancel the operation of mounting
> > >  a drive or volume.  So if you cancel the handle from
> > >  gtk_file_system_volume_mount(), your callback will not be called,
> > >  but the volume *will* be mounted (unless an error occurs during
> > >  mounting, of course).
> >
> > i really consider this scenrio a MUST-NOT-HAPPEN (sorry for the caps ;)
> > simply because it renders the _combination_ of two _vital_ features
> > unusable (unreliable), namely cancellation of operations and notification
> > about the success of operations.
>
> In the particular case of volume mounts, you really do not care if the
> operation happens even if you cancel it.  Canceling it simply means
> "don't call my callback because I'm dying soon".  The user will see a
> volume icon appear elsewhere in his desktop.

this is intermixing several different things:
a) file system operation cancellation;
b) success/error notification about async operations.
c) callback/memory maintenance as in disconnecting a signal handler
   or removing a main loop handler.

these three can't munged together into a single cancellation request without
loss. your suggestion goes at the expense of (b) and that's what gets you
started on having to argue about operations that may be cancelled but are
possibly not of interest (mounts) or may be cancelled and will leak (open)
but are hopefully not used by gtk any time soon...

the current scheme that was consensus when Kris' initial design was
discussed goes at the expense of (c), i.e. callback invokation can't be
undone by the caller. the rationale here is that information loss would
be worse and it's easy inside the callback to just return.

nobody suggested going at the expense of (a) so far, since that would
effectively just amount to callback removal and not cancel the async
operation. however, doing so while providing the ability to cancel the
async operation would result in an API that was something like:
  void gtk_file_system_cancel (GtkFileSystemHandle *handle,
                               gboolean             cancel_operation,
                               gboolean             disconnect_callback);
i.e. the operation and the callback could be cancelled (disconnected)
together or individually. for operations where you "normally don't care",
you can use TRUE,TRUE, for operations that still need aftermath, e.g.
closing a file handle after unsuccessfull cancellation of open(),
you use TRUE,FALSE, and in case you want to ignore further progress
during an ongoing operation, FALSE,TRUE.

but before some change like this really is decided on, could you please
point out actual _programming_ problems you're having with the
current scheme?
so far, Kris has been working on fixing the remaining issues your other
email pointed out, and hasn't complained about problems with the
cancellation mechanism yet...

> Remember than cancellation is there simply to
>
> a) change contexts.  The user got impatient and closed the file chooser
> window, or he went to another directory before the current one got
> loaded.
>
> b) make cleanup easy.  You don't want callbacks to have to worry that
> their surrounding context is dead.
>
> c) have the possibility of saving a little bit of work if (a) happens
> before the actual operation takes place in the worker thread.

well, what's with user-side cancellation here? e.g. if as a user i abort
a lengthy file copy or file removal, i'd certainly want to know if the
operation completed regardless. and your planned changes fail to provide
this information, not for a particular callback, but _by design_. big mistake.

> Not to sound patronizing, but I already rewrote the cancellation layer
> in GnomeVFS one time, so I know what I'm talking about ;)

well, i guess i could throw some weight in as well here, but that'd be
pointless, it just boils down to irrelevant "trust me" chatter:
  http://blogs.gnome.org/view/timj/2005/11/08/0


>  Federico

---
ciaoTJ