Re: Some comments about GVFS
Hi Alex, On Thu, 2007-05-10 at 09:41 +0200, Alexander Larsson wrote: > So, if you were to implement gstreamer sinks and sources for gvfs, an > api like: > gssize g_input_stream_read (GInputStream *stream, > void *buffer, > gsize count, > GCancellable *cancellable, > GError **error); > > Would be ok with you? Its a blocking call, but if you pass in a > GCancellable you can use that to cancel the call from another thread. I pinged this off a couple of other folks and we all think it's OK. Make sure the read never happens if the cancellable was cancelled before the read began, of course. > And you wouldn't be interesting in async (mainloop-using) i/o operations > at all? I do not see GStreamer taking advantage of this, no. Cheers, Andy. -- http://wingolog.org/ ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Wed, 2007-05-09 at 23:58 +0200, Andy Wingo wrote: > Hello, > > On Tue, 2007-05-08 at 16:59 +0200, Alexander Larsson wrote: > > I'm still not sure why gstreamer needs to run its own mainloop in its > > thread though. If its async, then it should be able to use the default > > mainloop. Is it because there is no guarantee of the default mainloop > > running always? > > I am not certain how the mainloop got involved here. GStreamer uses > threads so that it can use blocking read() and write() calls. > > Underneath, it's true they go through select() first on the fd + a > control fd from a socketpair, because you need to be able to cancel a > wait from another thread, so that you can seek, for example. Perhaps > that is the concern? > > In any case, as long as one is able to cancel an in-progress sync > operation from another thread, GStreamer is fine, AFAIK. So, if you were to implement gstreamer sinks and sources for gvfs, an api like: gssize g_input_stream_read (GInputStream *stream, void *buffer, gsize count, GCancellable *cancellable, GError **error); Would be ok with you? Its a blocking call, but if you pass in a GCancellable you can use that to cancel the call from another thread. And you wouldn't be interesting in async (mainloop-using) i/o operations at all? =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander LarssonRed Hat, Inc [EMAIL PROTECTED][EMAIL PROTECTED] He's an impetuous small-town cat burglar on the wrong side of the law. She's a vivacious mutant fairy princess with a knack for trouble. They fight crime! ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
Hello, On Tue, 2007-05-08 at 16:59 +0200, Alexander Larsson wrote: > I'm still not sure why gstreamer needs to run its own mainloop in its > thread though. If its async, then it should be able to use the default > mainloop. Is it because there is no guarantee of the default mainloop > running always? I am not certain how the mainloop got involved here. GStreamer uses threads so that it can use blocking read() and write() calls. Underneath, it's true they go through select() first on the fd + a control fd from a socketpair, because you need to be able to cancel a wait from another thread, so that you can seek, for example. Perhaps that is the concern? In any case, as long as one is able to cancel an in-progress sync operation from another thread, GStreamer is fine, AFAIK. Regards, Andy. -- http://wingolog.org/ ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Tue, 2007-05-08 at 11:19 +0200, Benjamin Otte wrote: > (short reply here, so you can continue your interesting discussion, > I'll answer in more detail when I'm back from UDS) > > On 5/8/07, Alexander Larsson <[EMAIL PROTECTED]> wrote: > > On Tue, 2007-05-08 at 03:33 -0500, Hans Petter Jansson wrote: > > > > I know Benjamin mentioned multiple contexts was needed for gstreamer > > integration. It would be nice to hear some details about this > > > GStreamer runs the pipeline in seperate threads. While reads are > typically scheduled from just one thread, any thread could potentially > initiate actions on the stream. An example would be seeking: seek > events are dispatched from the thread doing the seek - this could be > the main thread for a user event or any other thread if a downstream > element needs to seek. So the event would cancel the read operation > and do the seek. This would wake up the read thread noticing it was > interrupted and reschedule the read. So, you would have one of gstreamers threads doing the read operation, and this would be using async i/o and a custom glib mainloop on that thread. Then say the main thread on user input decides to seek in the stream, this seek call on the main thread will cancel the async read operation running in the gstreamer thread and schedule a new seek operation there instead. Is this a correct example? I'm still not sure why gstreamer needs to run its own mainloop in its thread though. If its async, then it should be able to use the default mainloop. Is it because there is no guarantee of the default mainloop running always? =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander LarssonRed Hat, Inc [EMAIL PROTECTED][EMAIL PROTECTED] He's a short-sighted guitar-strumming assassin in drag. She's a foxy insomniac detective with a knack for trouble. They fight crime! ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
(short reply here, so you can continue your interesting discussion, I'll answer in more detail when I'm back from UDS) On 5/8/07, Alexander Larsson <[EMAIL PROTECTED]> wrote: > On Tue, 2007-05-08 at 03:33 -0500, Hans Petter Jansson wrote: > > I know Benjamin mentioned multiple contexts was needed for gstreamer > integration. It would be nice to hear some details about this > GStreamer runs the pipeline in seperate threads. While reads are typically scheduled from just one thread, any thread could potentially initiate actions on the stream. An example would be seeking: seek events are dispatched from the thread doing the seek - this could be the main thread for a user event or any other thread if a downstream element needs to seek. So the event would cancel the read operation and do the seek. This would wake up the read thread noticing it was interrupted and reschedule the read. Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Tue, 2007-05-08 at 10:57 +0200, Alexander Larsson wrote: > On Tue, 2007-05-08 at 03:33 -0500, Hans Petter Jansson wrote: > > I'm aware of the dangers - when I worked on Evolution, this caused > > plenty of pain. > > > > However, it's useful in very simple apps where you have e.g. a "Sync" or > > "Connect" button which desensitizes most of the UI while the operation > > is in progress, only updating a progress bar or allowing the user to hit > > "Cancel". That's what I meant by a "program that lends itself to that > > design" above :) > > > > So I dunno. Maybe it's too seductive and should be eliminated as an > > option. > Avoiding this problem is one of the primary reasons of going from a > stateless model (where you can get auth callbacks anytime) to a > statefull model (auth only on mount). However, that problem in gnome-vfs > happens for *all* apps, not only those that "lend itself to that > design", so its a larger problem there. Agree. -- Hans Petter ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Tue, 2007-05-08 at 03:33 -0500, Hans Petter Jansson wrote: > I'm used to three main design patterns: > > 1) Everything in one thread with sync I/O blocking the UI. Simple code > but unacceptable performance. > > 2) Everything in one thread with async I/O using callbacks. Fine for > simple protocols or if you prefer an explicit state machine. > > 3) Doing synchronous I/O operations in one or more subthreads that talk > to the main thread using asynchronous messages queues. This is nice for > complex protocol implementations, because synchronous code is easier to > understand and maintain. These are the main model i see too. But none of these need any main context but the default mainloop one... I know Benjamin mentioned multiple contexts was needed for gstreamer integration. It would be nice to hear some details about this > I'm aware of the dangers - when I worked on Evolution, this caused > plenty of pain. > > However, it's useful in very simple apps where you have e.g. a "Sync" or > "Connect" button which desensitizes most of the UI while the operation > is in progress, only updating a progress bar or allowing the user to hit > "Cancel". That's what I meant by a "program that lends itself to that > design" above :) > > So I dunno. Maybe it's too seductive and should be eliminated as an > option. Avoiding this problem is one of the primary reasons of going from a stateless model (where you can get auth callbacks anytime) to a statefull model (auth only on mount). However, that problem in gnome-vfs happens for *all* apps, not only those that "lend itself to that design", so its a larger problem there. =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander LarssonRed Hat, Inc [EMAIL PROTECTED][EMAIL PROTECTED] He's an ungodly native American paramedic on the hunt for the last specimen of a great and near-mythical creature. She's a wealthy mutant opera singer from the wrong side of the tracks. They fight crime! ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Tue, 2007-05-08 at 09:30 +0200, Alexander Larsson wrote: > On Mon, 2007-05-07 at 14:08 -0500, Hans Petter Jansson wrote: > > In Flow, I keep a GMainContext associated with each GThread. The API > > user can set his own GMainContext for each thread, but for simplicity it > > can only be set once, before any other Flow calls are made in that > > thread. > So, an async operation inherits the main context from the thread that > started it. Its nice that you don't have to fill up the API with context > arguments, but it also seems to make it a bit more complicated to handle > multiple contexts. For instance, you can't have one thread starting > async operations that run on multiple context. But perhaps this is not > so important? What do users of multiple contexts need? Being able to provide a different main context on each call is extremely flexible, but in the end I doubt it is necessary in the majority of apps, and as you pointed out, it clutters the code. The main reason I have the main-context-per-thread is that a Flow pipeline won't run without a main context - for instance, elements can limit flow rate by installing a timeout and resuming when it fires, or otherwise generate timed events. And applications will definitely want to run pipelines in threads. I'm used to three main design patterns: 1) Everything in one thread with sync I/O blocking the UI. Simple code but unacceptable performance. 2) Everything in one thread with async I/O using callbacks. Fine for simple protocols or if you prefer an explicit state machine. 3) Doing synchronous I/O operations in one or more subthreads that talk to the main thread using asynchronous messages queues. This is nice for complex protocol implementations, because synchronous code is easier to understand and maintain. All of the above can be done with I/O dispatched in a single GMainContext per thread. 1) is a special case where you could have one GMainContext for the UI and another for the I/O. That said, it'd be interesting to know if anyone runs multiple main contexts in a single thread and needs to dispatch I/O events in more than one of those. It sounds rather contrived to me... > > I find that this allows for a lot of flexibility without adding much > > code complexity at all: > > > > 1) The default GMainContext for the main thread is the default GLib > > GMainContext. This allows sync operations to spin the main loop (so if > > you have a program that lends itself to that design, you can process > > redraws and UI operations while in a sync I/O call). > Danger will robinson! This sounds like a recipe for disaster. Sync calls > calling back into the mainloop can easily cause reentrancy, and we're > back in bonobo hell again. I.E. call any flow function, and you could > reenter (via a mainloop callback) into any random code in the app which > could modify the state of the code that was calling flow. (And if that > state is in a partially changed state things can go very bad.) > > Its also very problematic wrt locking. We have the same problem with > authentication callbacks in gnome-vfs. These are called back from any > sync operation when authentication is needed, and often requires UI to > finish, and thus a mainloop. However, since it is doing UI it needs to > take the gdk lock, but that deadlocks if the gdk lock was taken when the > sync call was called. In the case of flow, what if you get an idle > callback inside a sync call, and the idle callback (correctly) takes the > gdk lock... I'm aware of the dangers - when I worked on Evolution, this caused plenty of pain. However, it's useful in very simple apps where you have e.g. a "Sync" or "Connect" button which desensitizes most of the UI while the operation is in progress, only updating a progress bar or allowing the user to hit "Cancel". That's what I meant by a "program that lends itself to that design" above :) So I dunno. Maybe it's too seductive and should be eliminated as an option. -- Hans Petter ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Mon, 2007-05-07 at 14:08 -0500, Hans Petter Jansson wrote: > On Mon, 2007-05-07 at 09:24 +0200, Alexander Larsson wrote: > > On Fri, 2007-05-04 at 17:52 +0200, Benjamin Otte wrote: > > > > Yeah, I missed those implementations as I was only grepping for > > > GMainContext which you don't use. > > > I think it's a good idea to make the main context customizable so gvfs > > > can be used from other threads. > > > The api used to have this, but there was a lot of overhead code carrying > > this around and adding it to the parameter list of all functions, and I > > thought there would be very very few people using it, so i killed it. > > Maybe we should discuss reverting that decision... > > In Flow, I keep a GMainContext associated with each GThread. The API > user can set his own GMainContext for each thread, but for simplicity it > can only be set once, before any other Flow calls are made in that > thread. So, an async operation inherits the main context from the thread that started it. Its nice that you don't have to fill up the API with context arguments, but it also seems to make it a bit more complicated to handle multiple contexts. For instance, you can't have one thread starting async operations that run on multiple context. But perhaps this is not so important? What do users of multiple contexts need? > I find that this allows for a lot of flexibility without adding much > code complexity at all: > > 1) The default GMainContext for the main thread is the default GLib > GMainContext. This allows sync operations to spin the main loop (so if > you have a program that lends itself to that design, you can process > redraws and UI operations while in a sync I/O call). Danger will robinson! This sounds like a recipe for disaster. Sync calls calling back into the mainloop can easily cause reentrancy, and we're back in bonobo hell again. I.E. call any flow function, and you could reenter (via a mainloop callback) into any random code in the app which could modify the state of the code that was calling flow. (And if that state is in a partially changed state things can go very bad.) Its also very problematic wrt locking. We have the same problem with authentication callbacks in gnome-vfs. These are called back from any sync operation when authentication is needed, and often requires UI to finish, and thus a mainloop. However, since it is doing UI it needs to take the gdk lock, but that deadlocks if the gdk lock was taken when the sync call was called. In the case of flow, what if you get an idle callback inside a sync call, and the idle callback (correctly) takes the gdk lock... =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander LarssonRed Hat, Inc [EMAIL PROTECTED][EMAIL PROTECTED] He's a fiendish skateboarding cyborg She's a mistrustful motormouth angel living on borrowed time. They fight crime! ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Mon, 2007-05-07 at 09:24 +0200, Alexander Larsson wrote: > On Fri, 2007-05-04 at 17:52 +0200, Benjamin Otte wrote: > > Yeah, I missed those implementations as I was only grepping for > > GMainContext which you don't use. > > I think it's a good idea to make the main context customizable so gvfs > > can be used from other threads. > The api used to have this, but there was a lot of overhead code carrying > this around and adding it to the parameter list of all functions, and I > thought there would be very very few people using it, so i killed it. > Maybe we should discuss reverting that decision... In Flow, I keep a GMainContext associated with each GThread. The API user can set his own GMainContext for each thread, but for simplicity it can only be set once, before any other Flow calls are made in that thread. I find that this allows for a lot of flexibility without adding much code complexity at all: 1) The default GMainContext for the main thread is the default GLib GMainContext. This allows sync operations to spin the main loop (so if you have a program that lends itself to that design, you can process redraws and UI operations while in a sync I/O call). 2) If you want hard blocking sync I/O calls, just set a different GMainContext for the thread, and don't use it for anything else. 3) You can get at the GMainContexts used in subthreads, so you can e.g. add your own timeouts or whatever to those. And the best thing is, you never have to pass the GMainContexts around. The context-per-thread management API is tiny: http://svn.gnome.org/viewcvs/flow/trunk/flow/flow-context-mgmt.h?revision=154&view=markup The only thing that can cause trouble is that in Flow code proper, I have to be careful to use flow_get_main_context_for_current_thread () instead of g_main_context_default (). Ditto for idle and timeout convenience functions. But I consider that minimal cost. -- Hans Petter ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Fri, 2007-05-04 at 17:52 +0200, Benjamin Otte wrote:
> > > 1b) Cancelling operations from another thread doesn't look like good
> > > design to me. I've learnt (both in theory and in practice) that
> > > threads are supposed to be independant and not call into each other.
> >
> > Eh? How else would you cancel a blocking i/o call? From within the
> > blocked thread? This is just bogus, cancellation is all about
> > cross-thread operations.
> >
> I'd argue that if you do a blocking call, you're aware that it's
> blocking and don't want to cancel it. Otherwise you'd use async I/O
> with proper cancellation mechanism such as g_main_context_wakeup().
> You've told me that it's main use case are transactions like calling
> the equivalent of gnome_vfs_xfer_async () which you'd want to
> implement in a thread by calling lots of sync operations one after
> another.
> I can see the use case, but even though I still don't like it, I can't
> come up with a better model.
Take for instance the default async i/o implementation (the one used for e.g.
local files). It uses threads and blocking calls. If we want the default async
implementation to support cancellation, then the sync version must support
cancellation too.
> Yeah, I missed those implementations as I was only grepping for
> GMainContext which you don't use.
> I think it's a good idea to make the main context customizable so gvfs
> can be used from other threads.
The api used to have this, but there was a lot of overhead code carrying
this around and adding it to the parameter list of all functions, and I
thought there would be very very few people using it, so i killed it.
Maybe we should discuss reverting that decision...
> I'd love if GVFS would advocate the async model over the sync model
> but provide both. So g_input_stream_read would be async and
> g_input_stream_read_async would exist, too.
> The reason for this is that I think in most cases you want the async
> behaviour and it helps to tell lazy programmers that this is the right
> way to go. It's purely psychological.
I dunno. Its contrary to what all other APIs do, and i'm not sure the
psychological advantage is all that great. Programmers generally need a
specific functions, and they'll look around until they find it. Who are
we to decide for them if sync or async is the better model for them.
> I think that solution is fine. However, there is one thing I am
> missing: The read_all_available_data_right_now() function call. Or at
> least a read_x_bytes_if_available() call. This is interesting because
> in some cases I want to avoid calling back into the main loop.
> It's an issue in Swfdec with gnome-vfs where I'm supposed to display
> how many percent of the file is loaded and that display gets updated
> via the main loop. So after every read () of DEFAULT_SIZE I get my
> display updated. So loading seems slow even though it isn't, just
> because every read goes via the main loop.
The openoffice stream class has a call that gets the number of bytes you
can currently read without blocking. Maybe something like that can be
added? For many streams this is not really something you can generally
calculate in a sane way though. Take a stream to a file on smb or nfs
for example. The way a read call works there is that you send a request
for read on N bytes at offset O, and you get back a reply with the data.
So, this function would always return zero.
And actually, this model is very similar to what the gvfs client code
does with the gvfs daemon, so it will be true for all vfs streams.
Automatic readahead can make it different, so can reading in larger
blocks, but the operation is still not quite what you expect.
Btw, how do you handle buffer management with a call like
read_all_availible_data_right_now? You have to pass in a buffer of some
size. When you do, how is this different from the normal read call? That
already returns immediately when it has read any data and further i/o
would block.
> > > 8) The API seems very forgiving to (IMO) obvious porogramming
> errors
> > > that should g_return_if_fail. g_input_stream_read for example provides
> > > a proper error message when there's already a pending operation or
> > > when the size parameter is too large.
> >
> > Is this such a bad thing? Should we convert these to asserts?
> >
> I would advocate this. For one it's not an error message a user should
> be presented with ("Hey, someone passed a too large value to read()").
> But it's even more interesting when you stick the error into the
> object as I'm advocating above.
Yeah, this is probably a good idea.
> Another thing that came to my mind is if it would be better to use
> signals instead of providing callbacks for some operations. It seems
> somewhat weird for read(), but open and in particular close might be
> interesting to be implemented using signals.
I don't think signals are easier for one-shot function call initiated
callbacks like that. It just means you have to type mo
Re: Some comments about GVFS
On Fri, 04 May 2007 10:27:34 -0400, Havoc Pennington wrote: > It's a run-time env variable. I think we don't advertise it at all, it's > more of a defense against people who have strong views on the topic and > prefer "undefined behavior probably a crash" to "just exit" ... And one thing to note here is that cairo does provide an additional option: "No behavior, will not crash" which I think can form a good compromise between the camps that prefer each of the options you mention. (Though I'll agree that debugging would be easier if cairo _also_ provided a run-time mechanism to get the "just exit" along with an error message giving as much context as possible for the time of the error.) The point is, while it might not make sense to talk about error recovery from any single low-level library operation, (see your "failed to draw a rectangle" example), applications still will very much want to have high-level behavior of "something has gone very wrong, let's save the user's important state before exiting". There are a few things a library could do that would defeat that almost entirely: 1. Exit 2. Start doing undefined things 3. Require error-checks on every call into the library to prevent it from doing undefined things You seem to be saying that it's impractical for a library to avoid 2 or 3, so there's no reason not to do 1. But I think with cairo we've got a practical approach for getting close enough to avoiding all three so that applications really can do high-level error recovery. And that seems useful. > When I first implemented this it found quite a few bugs, I then > implemented something similar for part of libxml and also found a lot, > so that's why my party line since has been that trying to handle OOM > without testing the OOM codepaths is more hopeful fantasy than anything > else. I absolutely agree that testing is essential, and out-of-memory handling is doomed without it. And now that we've fixed so much in cairo, we really want to go fix underlying libraries like fontconfig and freetype or else it's really all in vain. > (it's just too hard to get right without testing - in some cases > in dbus-daemon I had to implement fairly complex "transactions" that > could be rolled back, in other cases I had to change the library API to > allow e.g. callbacks to signal OOM) When we started testing cairo, we found several mistakes, (though fewer than I had anticipated to tell the truth). But we've had nothing come even close to needing any API changes. Part of that is certainly due to cairo's problem domain where there aren't a lot of complex interactions between separate operations, (can anyone imagine transactions being useful when drawing?). But another part comes from just accepting support for OOM-error- handling as an upfront design constraint, rather than hoping to bolt it on later. -Carl pgpDjQ9MQtXgn.pgp Description: PGP signature ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
I just had a chat with Alex on IRC. (I have attached the log if anyone wants to read it.) Some of my ideas have changed after this discussion, so I might contradict my previous mail now. Bear with me. I've come to understand now that Alex' and my idea for what a stream is and how it should work are very different because we have very different use cases. Alex comes from the File manager world. For him GVFS is primarily a way to interact with "files" all over the world, rename them, tag them, and access it's contents. For me it's all about getting a stream to/from somewhere and write/read data to/from it. I'm wondering is if there are use cases for an I/O library that are different from those 2 use cases. During the discussion the question of how to write an async version of g_file_get_contents came up. (That's the paste link in the log.) I have attached my quick'n'dirty showcase of how I'd imagine that would work here, too. The git repo from today contains an implementation from Alex. That one actually works. So here's some followup comments. I'd like to note that what I care most about, because that's important for a nice API is point 7 below. > 1b) Cancelling operations from another thread doesn't look like good > design to me. I've learnt (both in theory and in practice) that > threads are supposed to be independant and not call into each other. Eh? How else would you cancel a blocking i/o call? From within the blocked thread? This is just bogus, cancellation is all about cross-thread operations. I'd argue that if you do a blocking call, you're aware that it's blocking and don't want to cancel it. Otherwise you'd use async I/O with proper cancellation mechanism such as g_main_context_wakeup(). You've told me that it's main use case are transactions like calling the equivalent of gnome_vfs_xfer_async () which you'd want to implement in a thread by calling lots of sync operations one after another. I can see the use case, but even though I still don't like it, I can't come up with a better model. > 2) GVFS seems to avoid the glib main loop for asynchronous operations > in favour of relying on threads. I'm not sure this provides any > benefits besides making apps way harder to debug. Is there a reason > for that? I wouldn't say it does. It gives you both sync (usefull if you do threads) and async operations (useful it your typical gui app). Almost all GUI apps are single threaded and async by desing, and integrating glib-style async i/o into such apps is generally *much* easier than manually mucking about with threads. However, the default async operations are implemented using threads, so in fact apps are often using threads already when using gvfs. Just in a way that fits well with a mainloop based gui app. And when doing async is *much* easier than using sync i/o we can avoid the thread cost by having a custom async implementation. (For instance the gvfs daemon protocol is very easy to do async without using threads.) Yeah, I missed those implementations as I was only grepping for GMainContext which you don't use. I think it's a good idea to make the main context customizable so gvfs can be used from other threads. There is a default implementation of async operations using the sync operations and threads. I don't think that means the model is only sync i/o. I think it gives equal width to both sync and async models. I'd love if GVFS would advocate the async model over the sync model but provide both. So g_input_stream_read would be async and g_input_stream_read_async would exist, too. The reason for this is that I think in most cases you want the async behaviour and it helps to tell lazy programmers that this is the right way to go. It's purely psychological. In fact, I agree with Linus (the syslet/threadlet thread on lkml is very interesting wrt to this argument) that there are many operations that just fit very badly with the asynchronous model, and things like pathname lookup and complicated filename operations are examples of that. The relevant email can be found at http://lkml.org/lkml/2007/2/25/177 - the followup and previous mails might be interesting, too. I need ot digest this before commenting on it though. There are two basic models when it comes to async i/o. I call them "push" and "pull". The "push" model is when you initiate a transfer, and then you get a new event each time there is more data availible. The "pull" model is that you initiate each read and then get an even when data for that read is availible. The main difference for the two models are: * flow control The push model doesn't give you much flow control. If you're copying from a fast source to a slow destination you will fill up memory with the data while waiting for the destination to accept your first block of data. You might also not really want to read the whole file (in for instance filetype sniffing), and this gets more complicated with push. * pipelining The pull model means you
Re: Some comments about GVFS
Hi, Carl Worth wrote: > Anyway, I've learned a few new things, and found the discussion very > useful. I hope you have as well. Certainly. > By the way, is that a compile-time or a run-tim environment variable? > And what measures do you take to advertise it? It's a run-time env variable. I think we don't advertise it at all, it's more of a defense against people who have strong views on the topic and prefer "undefined behavior probably a crash" to "just exit" ... > How does your D-Bus unit testing arrange to test all the OOM code > paths? I have a dbus_malloc wrapper and it can be told to fail the Nth malloc. The test suite runs once and counts mallocs, then it runs over and over and over failing mallocs 1, 2, 3, 4, 5... ;-) Brute force! When I first implemented this it found quite a few bugs, I then implemented something similar for part of libxml and also found a lot, so that's why my party line since has been that trying to handle OOM without testing the OOM codepaths is more hopeful fantasy than anything else. (it's just too hard to get right without testing - in some cases in dbus-daemon I had to implement fairly complex "transactions" that could be rolled back, in other cases I had to change the library API to allow e.g. callbacks to signal OOM) > What we're doing in cairo, (only fairly recently, and thanks to Chris > Wilson), is to run cairo's standard test suites but with a valgrind > skin that Chris wrote to progressively inject malloc failures. The > skin is quite general and is described here: > > http://lists.freedesktop.org/archives/cairo/2007-April/010343.html Very nice. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Fri, 2007-05-04 at 15:27 +0200, Mathieu Lacage wrote: > > > 5) You seem to use void * in as the data pointers. All applications I > > > know use guchar * (some use gchar *) to handle data. From my stream > > > handling experience this is to encourage people to think about what > > > they pass to such a function. This seems to encourage calling > > > functions like this: write (mywidget, sizeof (MyWidget)) - with is a > > > bad idea for multiple reasons, including but not limited to struct > > > padding. MS formats seem to have done that a lot. > > > > I'm not partial to any format here. Do people thing that using a char > > type rather than a void type is better? > > I tend to use uint8_t * because I rely on stdint.h: this makes it > totally unambiguous that you are dealing with bytes, IMHO. I like to > avoid char * whenever I can because I have gotten more than once into > portability problems due to the fact that the signedness of char is > different on different platforms. If I remember correctly one of the > embedded platforms I used to work on (ARM or MIPS, I cannot recall at > the moment) was unsigned char while x86 is usually signed char. for unformatted blobs of data, the convention in GLib/GTK+ appears to be using guchar*, to avoid the signedness issue like you reported; see the GtkSelectionData and the drag and drop API in GTK+ and the base64 API in GLib for precedents of that. ciao, Emmanuele. -- Emmanuele Bassi, E: [EMAIL PROTECTED] W: http://www.emmanuelebassi.net B: http://log.emmanuelebassi.net ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Fri, 2007-05-04 at 11:40 +0200, Alexander Larsson wrote: > > 5) You seem to use void * in as the data pointers. All applications I > > know use guchar * (some use gchar *) to handle data. From my stream > > handling experience this is to encourage people to think about what > > they pass to such a function. This seems to encourage calling > > functions like this: write (mywidget, sizeof (MyWidget)) - with is a > > bad idea for multiple reasons, including but not limited to struct > > padding. MS formats seem to have done that a lot. > > I'm not partial to any format here. Do people thing that using a char > type rather than a void type is better? I tend to use uint8_t * because I rely on stdint.h: this makes it totally unambiguous that you are dealing with bytes, IMHO. I like to avoid char * whenever I can because I have gotten more than once into portability problems due to the fact that the signedness of char is different on different platforms. If I remember correctly one of the embedded platforms I used to work on (ARM or MIPS, I cannot recall at the moment) was unsigned char while x86 is usually signed char. regards, Mathieu -- ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Thu, 2007-05-03 at 11:11 +0200, Benjamin Otte wrote:
> Hi,
>
> I had an initial look at gvfs in particular the Inputstream and
> Outputstream implementations, and some comments came to my mind, in
> particular about the API. So I thought I'd post them as early as
> possible.
> These comments are about the code in
> http://www.gnome.org/~alexl/git/gvfs.git from today - May 2nd.
> I should also note that these ideas are probably heavily influenced by
> where I'm coming from - namely GStreamer and Swfdec - which involve
> handling low level byte streams possibly caring about low latency. In
> this particular case I had been wondering about making the basic
> input/output objects in Swfdec GInput/OutputStream.
>
> 1) I don't like GCancellable This is for several reasons:
>
> 1a) GCancellable is far too visible. It's present in every function
> call, so it must be very important. Maybe exporting
> g_push/pop_current_cancellable () would be a better API because it
> decouples the cancelling from the actual operations?
There are two cases here. For async operations we clearly need to pass
the cancellable object to the operation, because we can't rely on a
thread-local storage setting over the lifetime of the async operations.
Now, for sync calls things are less obvious. At one time I had the
cancellable object as an implicit argument (via tls). However, I ran
into trouble with that and had to be explicit about it. I'm trying to
recall the details of that atm... I think the problem was that if your
implementation of an operation relies on other gio operations those
would always inherit the cancellable of the outer operation. This isn't
always what you want, as there are casese where you need to do a
non-cancellable i/o as part of a larger cancellable operation (an
example is that you don't want to send partial messages and mess up the
protocol state in the daemon backend, instead you want to send a cancel
request).
Maybe there were other reasons too, because it seems like this one would
be fixable if one allows to stack a NULL cancellable over a non-NULL
one.
> 1b) Cancelling operations from another thread doesn't look like good
> design to me. I've learnt (both in theory and in practice) that
> threads are supposed to be independant and not call into each other.
Eh? How else would you cancel a blocking i/o call? From within the
blocked thread? This is just bogus, cancellation is all about
cross-thread operations.
> 2) GVFS seems to avoid the glib main loop for asynchronous operations
> in favour of relying on threads. I'm not sure this provides any
> benefits besides making apps way harder to debug. Is there a reason
> for that?
I wouldn't say it does. It gives you both sync (usefull if you do
threads) and async operations (useful it your typical gui app). Almost
all GUI apps are single threaded and async by desing, and integrating
glib-style async i/o into such apps is generally *much* easier than
manually mucking about with threads.
However, the default async operations are implemented using threads, so
in fact apps are often using threads already when using gvfs. Just in a
way that fits well with a mainloop based gui app. And when doing async
is *much* easier than using sync i/o we can avoid the thread cost by
having a custom async implementation. (For instance the gvfs daemon
protocol is very easy to do async without using threads.)
Of course, threads have their place too. For instance, a complicated set
of file operations like a recursive directory copy is imho best
implemented using sync calls in a single thread. And gvfs is set up to
handle this case well too.
> 3) The whole API seems to be built around the model of synchronous
> operation. (I think so because of this code comment: /* Async ops:
> (optional in derived classes) */) I always thought everyone agrees
> that synchronous operation should be a thing of the past. and only be
> supported as an add-on for lazy coders or really simple apps.
> Almost everyone implements synchronous operations something like this:
> void foo () { while (errno == EAGAIN) foo_async (); };
> The kernel sure does.
There is a default implementation of async operations using the sync
operations and threads. I don't think that means the model is only sync
i/o. I think it gives equal width to both sync and async models.
I also thing you are naive if you think sync i/o is a thing of the past
and only for "lazy" coders. There are many cases where asynchronous i/o
is way to complicated and doesn't give any advantages over sync i/o.
(Like the above recursive copy operation, i dare you to implement
gnome_vfs_xfer using only asynchronous primitives!).
In fact, I agree with Linus (the syslet/threadlet thread on lkml is very
interesting wrt to this argument) that there are many operations that
just fit very badly with the asynchronous model, and things like
pathname lookup and complicated filename operations are examples of
that.
> 4) The asynchronous API seems t
Re: Some comments about GVFS
On Thu, 03 May 2007 16:52:39 -0400, Havoc Pennington wrote: > Carl Worth wrote: > "cairo model" is probably confusing - write_to_png is a more typical and > essentially GError-equivalent model, while "cairo_t model" is the thing > that people mean is unique to Cairo. Yeah, we can get in a lot of confusion here whether we're talking about the merits of the approach in any particular library vs. the idea of applying that approach to new things. Anyway, I've learned a few new things, and found the discussion very useful. I hope you have as well. > Of course. The case where this would break is an object with its own > locks, e.g. DBusConnection. I'm just guessing that gvfs has its own > locks since I believe gnome-vfs did. Yes, internal locks would definitely change things a lot. I just generally don't believe in such locks, but that would be a whole separate thread... ;-) > There is an official GLib/GTK+ policy on this, which is that g_assert is > used only for bugs in the library, and return_if_fail is used for bugs > in the app. It is missing from the docs indeed. OK, that makes sense. > > I could imagine a consistent argument that all programmer errors > > should lead to an abort, (to force errors to get noticed and dealt > > with early). > > D-Bus in fact does this unless you disable it by env variable. It is > pretty controversial, though. Or at least there's a vocal group who > doesn't like it. > > Obviously I think said group is wrong ;-) And that makes a _lot_ of sense. I was a little confused with your rationale about programmer errors and then the discussion of g_return_if_fail. By the way, is that a compile-time or a run-tim environment variable? And what measures do you take to advertise it? > The big differences are 1) that it prints a message and 2) that all > behavior is undefined after a return_if_fail - GTK doesn't contain > handling code that frees resources or tries to recover or puts objects > in a no-op state as a result of the return_if_fail. Yup. I'd be glad to print in cairo if the programmer has asked for undefined behavior, but in that case I might as well just abort. > I don't think 2) really matters to library users, it only affects > library maintainer effort. Fair enough. My experience is that there's not much maintainer effort involved here. Implementing the no-op behavior is no harder than the moral equivalent of a single g_return_if_fail at the beginning of every entry point. And once you've got the no-op it's really easy to provide defined behavior without much extra code, (we just don't care what the state of the object is, except that it's destroy function should still work). > Well, if and only if the error is a programming error. That's the > problem, I don't want noisy in write_to_png, nor do I want noisy if the > error is something I should be handling or is something I'm legitimately > ignoring. I only want noisy about bugs. Fair enough. I think I could be convinced to accept code to make cairo fail with a message when the user asks for some undefined behavior. > What D-Bus does in a couple of cases (and GTK may have one or two > examples of this also, iirc) is that if you pass NULL to ignore the > DBusError/GError, it is noisy, and if you don't pass NULL it is not > noisy. However, this is only OK if passing NULL when an error occurs > amounts to a programming bug, i.e. when you are only allowed to pass > NULL if you know errors are impossible. If it would be legitimate and > normal for the error to be silently ignored in some cases, you can't be > verbose when one happens. I can't actually find an example in the code > quickly, but I swear we do this in a couple places. The stuff described in that paragraph seems ver confusing. :-) > For example say you pass nonsense values; the warning can be > "function_name: assertion failed: arg_foo != 0.0" so I look at my calls > to function_name where arg_foo might be 0.0. Like I said, I could see adding that, (and likely with an abort by default). The hardest part I think would be doing a good job about advertising this and advertising how to make it go away. > With cairo_t drawing code, you have to add temporary code to check the > status and print errors, then to figure out which function sets the > error you have to keep moving this temporary code around, then you have > to figure out what the status means once you know which function sets > the error status. Oh, yuck. You really should have complained to me about something like that. Nobody should ever have to keep moving temporary error-checking code around to find their mistake. That's awful. > Setting a break on _cairo_error is certainly easier (thanks for that > tip) but the reason I've never done that is that I did not know cairo > had an internal symbol that was always used to set errors ;-) for the > analogous situation in gtk, gdk_x_error, the warning when an X error is > received has something in it like "try setting a break on gdk_x_error" >
Re: Some comments about GVFS
Hi, Havoc Pennington wrote: > In GLib/GTK it's library bugs vs. app bugs, D-Bus maintains the same > distinction for _dbus_return_if_fail vs. _dbus_assert *but* D-Bus makes > them both fatal so it doesn't matter much anyhow. > And as Benjamin pointed out, D-Bus ships with assertions disabled in stable releases, so it can have loads and loads of them in devel snapshots. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
Hi, Carl Worth wrote: > Sure. If you want the object to remain usable after an error, then it > shouldn't shut down. And note that that with cairo, we do have > functions that do return an error indication instead of shutting down, > (cairo_surface_write_to_png, for example). > > So I think a fair characterization of the "cairo model" is that it > recognizes that you don't always do the shutdown-on-error thing, but > it is darn handy when it's the right thing to do. "cairo model" is probably confusing - write_to_png is a more typical and essentially GError-equivalent model, while "cairo_t model" is the thing that people mean is unique to Cairo. > For cairo, at least, there's nothing broken here. If you want to use > the same object from multiple threads, then you need to be doing your > own locking. Of course. The case where this would break is an object with its own locks, e.g. DBusConnection. I'm just guessing that gvfs has its own locks since I believe gnome-vfs did. > First and foremost: GError should only be used to report > recoverable runtime errors, never to report programming > errors. If the programmer has screwed up, then you should use > g_warning(), g_return_if_fail(), g_assert(), g_error(), or > some similar facility. > > For that second sentence, if we're talking about a non-recoverable > programmer error, then what guidance is there for choosing > g_return_if_fail as opposed to g_assert? Are programmer errors > actually divided into two classes? There is an official GLib/GTK+ policy on this, which is that g_assert is used only for bugs in the library, and return_if_fail is used for bugs in the app. It is missing from the docs indeed. > I could imagine a consistent argument that all programmer errors > should lead to an abort, (to force errors to get noticed and dealt > with early). D-Bus in fact does this unless you disable it by env variable. It is pretty controversial, though. Or at least there's a vocal group who doesn't like it. Obviously I think said group is wrong ;-) > But how does g_return_if_fail fit into your model? Isn't it really > doing basically the same thing as cairo's inert object functions? > Differences I see are that: > > 1. g_return_if_fail prints a message > > 2. cairo's inert objects come with a guarantee that once an >object shuts down, no future call to that object will have >any effect. The big differences are 1) that it prints a message and 2) that all behavior is undefined after a return_if_fail - GTK doesn't contain handling code that frees resources or tries to recover or puts objects in a no-op state as a result of the return_if_fail. I don't think 2) really matters to library users, it only affects library maintainer effort. > So all you really want is a mode to make cairo be noisy on the first > detected error? Well, if and only if the error is a programming error. That's the problem, I don't want noisy in write_to_png, nor do I want noisy if the error is something I should be handling or is something I'm legitimately ignoring. I only want noisy about bugs. What D-Bus does in a couple of cases (and GTK may have one or two examples of this also, iirc) is that if you pass NULL to ignore the DBusError/GError, it is noisy, and if you don't pass NULL it is not noisy. However, this is only OK if passing NULL when an error occurs amounts to a programming bug, i.e. when you are only allowed to pass NULL if you know errors are impossible. If it would be legitimate and normal for the error to be silently ignored in some cases, you can't be verbose when one happens. I can't actually find an example in the code quickly, but I swear we do this in a couple places. > But a little noise isn't really enough information to tell you where > the error came from, so you're still going to have to track it > down. For example say you pass nonsense values; the warning can be "function_name: assertion failed: arg_foo != 0.0" so I look at my calls to function_name where arg_foo might be 0.0. With cairo_t drawing code, you have to add temporary code to check the status and print errors, then to figure out which function sets the error you have to keep moving this temporary code around, then you have to figure out what the status means once you know which function sets the error status. Setting a break on _cairo_error is certainly easier (thanks for that tip) but the reason I've never done that is that I did not know cairo had an internal symbol that was always used to set errors ;-) for the analogous situation in gtk, gdk_x_error, the warning when an X error is received has something in it like "try setting a break on gdk_x_error" even, or used to. > What you'd really like is a nice application-level stack trace > from when the error gets detected by cairo, (hopefully people using > bindings are getting this already). D-Bus does this on glibc systems - GNU libc
Re: Some comments about GVFS
On 5/3/07, Carl Worth <[EMAIL PROTECTED]> wrote: > So that looks to me like if you modified _cairo_error to do the > equivalent of g_assert, you'd basically get what you want for your > recoverable and unavoidable cases. But that would leave you with > always having g_assert and never g_return_if_fail for programming > errors though. So please explain more to me about how that handling > should be distinguished. > I've learned that the difference between a g_assert and a g_return_if_fail is whose fault it is. g_assert would be used for internal consistency checks inside cairo, where the cairo developers ensure they don't write bad code. The best example here is a case statement as in get/set_property functions, where it's common to add an assert in the default case. g_return_if_fail is for developers using your library. It's used to tell them that they are doing something wrong. It's basically the same thing, just catering to a different audience. In short: applications should have no call to g_return_if_fail. Everything should be g_assert. There's some advantages to this differentiation: 1) It allows you as a developer to disable internal assertions for various reasons (try running DBUS with all checks enabled and compare the speed) while still getting informed about faults in using the library. 2) It allows adding warnings later on. If people have been using your library wrong because they didn't know about something, you can add a g_return_if_fail to that function and the next time they'll run the program, they'll notice the warning and hopefully fix the bug. GThread has recently done something like this with printing a warning when g_thread_init() wasn't the first function called. There's some things that I don't like about the Glib/Gnome approach: 1) Assertions aren't disabled in releases. This leads to people often not putting enough assertions where it matters. Noone wants inner loops to be slow. DBUS does that differently and I think it has really helped developing DBUS. 2) g_return_if_fail returning often isn't useful and will lead to a crash. An example here is all those functions that have a GError argument and return FALSE on failure. They return FALSE in the return_if_fail case, too, but don't set the error. This of course causes a program to assume an error was set to crash nonetheless. It would probably be nice to just return TRUE. In general, I think being as annoying as possible when a developer does something wrong is a good thing, because people are lazy. So unless you force them to care about something, they won't and just hope it works anyway. So I could probably live fine with making every g_return_if_fail an assertion. But maybe I'm a bit extreme that way. I'm known to annoy people with -Werror. ;) Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Thu, 03 May 2007 15:41:43 -0400, Havoc Pennington wrote: > I'd be for that in the case that the API has a "required ending call," > the equivalent of close(). Then you can "force" error checking on that > last call with a GError or warn_if_unused approach and in theory people > could skip the intermediate error checks. That makes sense. Yes, forcing the user to check the return value when finished with the object is definitely a good idea. Aside for the public error-handling strategy, we've been adopting the same shutdown-on-error approach internally within cairo bit-by-bit. And as we do that, we really need to propagate errors from one object to another. And we've found that forcing a check when disposing of an object is a good way to help us not forget that. We've wondered whether we can change cairo_destroy from void to cair > > I'm not sure there are "unavoidable-and-ignorable" errors. > > To me anything that could happen to cairo_t is probably in this > category... what am I going to do if I draw a rectangle and it fails? Well, for "draw a rectangle", cairo basically says that that can never fail, (if you look at the X interfaces we get, there's no other option). So I think that's an error case that really doesn't exist, as opposed to something that's unavoidable and ignorable. Just look at cairo_status_t---there's nothing like "the requested drawing operation didn't happen" there. > really have no idea what kind of "recovery" code I'd write for that. > Certainly nobody is writing such code that I've seen. I don't even know > where the error check would go - in every widget's expose handler? In > the toplevel event loop? As you said before, at the very least at the "required ending call", (which is cairo_destroy in this case), you should definitely be checking this error. > Is there anything that could happen to cairo_t in this category? I don't > know. I'm mostly assuming there is because I have no idea what the > cairo_t error state is for otherwise ;-) (other than as an internal > "make everything a noop" flag) > > My guess is that out of memory (on client side or X server side) is in > this category in cairo, it silently doesn't draw if OOM occurs, no? There's certainly not a large class of errors that can happen while drawing, (yes, out of memory is one). But the other way you can get an error lodged into a cairo_t is via propagation from some other object. So, if you forget to check a cairo_surface_t's status, (say it's FILE_NOT_FOUND), and then you do "cairo_create(surface)" then the FILE_NOT_FOUND status will propagate into the cairo_t. It would have been kind of nice if GTK+ were creating the cairo_t for the expose handler and then also checking the status, and printing that message. But that's not how GTK+ integrates with cairo---oh, well. And, no, getting a FILE_NOT_FOUND status on a cairo_t is not going to give you a meaningful error message. And no, I wouldn't think it would be sane for anyone to right code to "handle" a FILE_NOT_FOUND status from a cairo_t. But it might actually give enough of a hint to help you know where to look, (or else just go and get a _cairo_error-induced stack trace). And, yes, when you find the cairo_image_surface_create_from_png call that's failing, (and was missing any errro checking), you should add code there to print a meaningful error message. > As long as the individual calls failing don't affect control flow, > there's some "close()" call that reminds you to grab the error, and the > errors are interesting/recoverable in the first place, I agree this is nice. That sounds quite reasonable to me. I don't think everyone will agree on which errors are or are not interesting. And I don't think it hurts at all to be able to fetch an uninteresting status value from an object, (see my example above of cairo_t reporting a FILE_NOT_FOUND status). -Carl pgpAjmYycGVjS.pgp Description: PGP signature ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On Thu, 03 May 2007 14:11:02 -0400, Havoc Pennington wrote: > Yeah, I think this keeps the Cairo model from being "fully general" - it > only works for some types of operations or objects. Sure. If you want the object to remain usable after an error, then it shouldn't shut down. And note that that with cairo, we do have functions that do return an error indication instead of shutting down, (cairo_surface_write_to_png, for example). So I think a fair characterization of the "cairo model" is that it recognizes that you don't always do the shutdown-on-error thing, but it is darn handy when it's the right thing to do. > Another thing about > the Cairo model is that it's either weird or possibly broken if an > object is used by multiple threads, which could arise in the current > gvfs presumably. For cairo, at least, there's nothing broken here. If you want to use the same object from multiple threads, then you need to be doing your own locking. As for weird, the shutdown thing should only be happening if there's really no other appropriate response. There's a difference between causing an error _in_ an object, (in which case it will shutdown, and it's appropriate for it to be shutdown for all threads), as opposed to causing an error _with_ an object, (in which case it should just return the error indication and not shutdown). > Another thing that's needed for gvfs for sure but not for Cairo perhaps > is the error message. The model of lodging errors inside the objects doesn't prevent you from putting as detailed a message in their as you'd like. With cairo itself, we don't actually store anything more than an enum, but... > 8859-1"), but nonetheless in a lot of cases I think a detailed error can > be better than a strerror-style string. Sometimes the error will be > user-helpful, e.g. "You aren't allowed to save to directory Foo" ... even with the very strerror-style approach that cairo encourages, it's very easy for the calling application to add context like this when generating its error message. > Returning *helpful* errors, even if only to programmers and techies, > also frequently depends on context - meaning, the error can be more > helpful if it's checked right away after every call. Sure. So everytime you want to emit a useful error message, check the status and generate the message with all the context you want. Or, like I said above, you could envision applying the cairo model and also adding a string to it. (That is, if applying the cairo model pushes the GError from a parameter to every function to instead be a field within the object, that doesn't mean that GErrror needs to become any less capable). > Finally, Cairo of course mixes programmer errors (g_return_if_fail in > GLib-world) with should-be-reported errors (GError in GLib-world) with > unavoidable-and-should-be-ignored errors (just get ignored in > GLib-world). See GError docs for how these are separated in GLib: > http://developer.gnome.org/doc/API/2.2/glib/glib-Error-Reporting.html I definitely agree with you that there are different classes of errors and they require different kinds of handling. And I won't argue that cairo gets all of this right yet. (In a very real sense I've always felt that cairo's error-handling strategy was a big experiment, and it would be interesting to see how it played out. So I'm quite glad to see this discussion happening around it.) In the meantime, it's not clear to me that either cairo or glib has this all figured out yet. For example, the document above says: First and foremost: GError should only be used to report recoverable runtime errors, never to report programming errors. If the programmer has screwed up, then you should use g_warning(), g_return_if_fail(), g_assert(), g_error(), or some similar facility. For that second sentence, if we're talking about a non-recoverable programmer error, then what guidance is there for choosing g_return_if_fail as opposed to g_assert? Are programmer errors actually divided into two classes? I could imagine a consistent argument that all programmer errors should lead to an abort, (to force errors to get noticed and dealt with early). And I could accept that some people would want to configure the abort away as well, (which G_DISABLE_ASSERT allows for example). So I could imagine a consistent argument that g_assert should be used for all detected programmer errors. But how does g_return_if_fail fit into your model? Isn't it really doing basically the same thing as cairo's inert object functions? Differences I see are that: 1. g_return_if_fail prints a message 2. cairo's inert objects come with a guarantee that once an object shuts down, no future call to that object will have any effect. > For me when using Cairo drawing I would say should-be-reported errors > aren't really possible. When the problem is that I passed
Re: Some comments about GVFS
Hi, Benjamin Otte wrote: > To bring up some examples: GdkPixbufLoaders, GKeyFile, GBookmarkFile, > GMarkup. I'd be for that in the case that the API has a "required ending call," the equivalent of close(). Then you can "force" error checking on that last call with a GError or warn_if_unused approach and in theory people could skip the intermediate error checks. That makes sense. > I'm not sure there are "unavoidable-and-ignorable" errors. To me anything that could happen to cairo_t is probably in this category... what am I going to do if I draw a rectangle and it fails? I really have no idea what kind of "recovery" code I'd write for that. Certainly nobody is writing such code that I've seen. I don't even know where the error check would go - in every widget's expose handler? In the toplevel event loop? Is there anything that could happen to cairo_t in this category? I don't know. I'm mostly assuming there is because I have no idea what the cairo_t error state is for otherwise ;-) (other than as an internal "make everything a noop" flag) My guess is that out of memory (on client side or X server side) is in this category in cairo, it silently doesn't draw if OOM occurs, no? You're right that if this class of error exists it's pretty rare. It would probably mean a bug or at least design bug in something, though possibly not your own app. > At least > I'm not very impressed by the error reporting of Gtk when X has > problems. (That one's probably mostly Xlib's fault though). Examples? Any recoverable error it should handle for you or report to you, the main unrecoverable error is BadAlloc which causes a g_error just like out of memory on the client side, and then some X errors are programming errors which GTK often tries to predict for you with return_if_fail but sometimes does not catch. Most X errors I would say are programming errors, the big exception is that if you are messing with another process's windows (as a WM does) there are lots of unavoidable but recoverable errors. Patches adding return_if_fail to avoid all the "programming error" Xlib errors would probably be accepted, is my guess. Xlib is definitely guilty of mixing up different kinds of error into one system that isn't appropriate or convenient for any kind of error... > And to say it once again: It's nice that the object carries the object > around with it. That way I don't have to carry the Error around myself > all the time as an extra argument. As long as the individual calls failing don't affect control flow, there's some "close()" call that reminds you to grab the error, and the errors are interesting/recoverable in the first place, I agree this is nice. Havoc ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
On 5/3/07, Havoc Pennington <[EMAIL PROTECTED]> wrote: > > Yeah, I think this keeps the Cairo model from being "fully general" - it > only works for some types of operations or objects. Another thing about > the Cairo model is that it's either weird or possibly broken if an > object is used by multiple threads, which could arise in the current > gvfs presumably. > [...] > For those reasons I don't agree at all with Benjamin that GError is the > "old" approach, I think it is still the right approach *for errors that > should generally be handled or recovered from in some way*. > I think the cairo style is the best way for recoverable errors that are fatal for the affected object. In that case you want to put the object into an error state and ignore further functions called on the object. To bring up some examples: GdkPixbufLoaders, GKeyFile, GBookmarkFile, GMarkup. All of those have a lot of functions (not all of them) that are fatal to the object and both the API and the code using it would look a lot nicer if there was one argument less. It may be interesting for some objects to have a function to recover from errors. > It's very very important IMO to distinguish the three types of errors > (programming, recoverable, and unavoidable-and-ignorable) when designing > any API in this area. > I'm not sure there are "unavoidable-and-ignorable" errors. At least I'm not very impressed by the error reporting of Gtk when X has problems. (That one's probably mostly Xlib's fault though). I agree with you that there are programming errors and recoverable errors, but not ignorable errors. Even cairo drawing errors should be somehow handled, and even if it's just printing to stdout. But almost all of those errors indicate a problem in your code (like cairo_scale (cr, 0.0, x)). The reason for allowing the programmer to check errors whenever he wants is to make programs of lazy people easier. We all know that good programs check errno after every call into libc, but most of us are lazy. And lazy people don't remember later that they passed NULL to g_key_file_get_double(). Nor do people that have to fix bugs. And to say it once again: It's nice that the object carries the object around with it. That way I don't have to carry the Error around myself all the time as an extra argument. Benjamin ___ gtk-devel-list mailing list gtk-devel-list@gnome.org http://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: Some comments about GVFS
Hi, Carl Worth wrote: > On Thu, 3 May 2007 11:11:39 +0200, "Benjamin Otte" wrote: >>I much prefer the cairo model, where basically the >> object keeps its own GError and has a function like cairo_status [3] >> that gives you the last error that occured. > > It's worth pointing out an additional aspect of the cairo > error-handling model: The object becomes entirely inert as soon as an > error occurs within it. That is, all methods of the object first check > the status and return immediately on error. > Yeah, I think this keeps the Cairo model from being "fully general" - it only works for some types of operations or objects. Another thing about the Cairo model is that it's either weird or possibly broken if an object is used by multiple threads, which could arise in the current gvfs presumably. Another thing that's needed for gvfs for sure but not for Cairo perhaps is the error message. A strerror()/errno type of thing is just not sufficient to give good errors from a system with pluggable backends. Even in Cairo, functions like cairo_surface_write_to_png return pretty unhelpful info on failure. Failure is uncommon, and the info would generally only be helpful to programmers not users anyway (gdk-pixbuf returns stuff like "Value for PNG text chunk cannot be converted to 8859-1"), but nonetheless in a lot of cases I think a detailed error can be better than a strerror-style string. Sometimes the error will be user-helpful, e.g. "You aren't allowed to save to directory Foo" Returning *helpful* errors, even if only to programmers and techies, also frequently depends on context - meaning, the error can be more helpful if it's checked right away after every call. errno obviously is an extreme example since it's garbage if not checked immediately. But say you do a series of gvfs operations then get a "no permissions" error, if the error message says which operation lacked permissions, that is much more helpful. strerror(EPERM) is not as good as "You aren't allowed to save to directory Foo" Finally, Cairo of course mixes programmer errors (g_return_if_fail in GLib-world) with should-be-reported errors (GError in GLib-world) with unavoidable-and-should-be-ignored errors (just get ignored in GLib-world). See GError docs for how these are separated in GLib: http://developer.gnome.org/doc/API/2.2/glib/glib-Error-Reporting.html For me when using Cairo drawing I would say should-be-reported errors aren't really possible. When the problem is that I passed nonsense to a Cairo API, I would prefer some verbose warning to the silent failure plus an error code I have to figure out the origin of. When the problem is something unavoidable (say not enough memory or whatever) then I just don't care, I'm not going to check the error code for this or in any way recover. So in neither case here do I find the error codes useful. I guess for some surfaces there might be a recoverable error I can imagine caring about, but for painting widgets in X, it just hasn't happened. The Cairo error reporting is something I simply don't use in connection with any drawing code, except in temporary code to figure out how I misused the API, where in GTK I'd instead have seen a warning right away. When the programmer _should_ check the error, e.g. cairo_surface_write_to_png, I think GError-style reporting is ideal, and the return code as write_to_png has is OK and does the job and is pretty much equivalent to GError except there's no memory management (plus) and no error message (minus). So to sum up: - cairo model is most usable when it can be correct to ignore errors, which is particularly common for programming errors (GLib would use return_if_fail/assert) or unavoidable runtime errors that are best handled by simply ignoring them - cairo model should perhaps be extended to support an error message for more general applications (since in the drawing case the idea is to almost always ignore errors, there's no point, but for file and network ops, there may well be) - cairo model raises issues when the error state is shared by multiple threads accessing one object - when it's really probably wrong to not check errors (e.g. when saving a file), a GError-type model with a function arg you must consider, or a return value with warn_unused_result as Carl mentions, is better than the cairo_t store-error-on-object model IMO - I personally believe programmer errors should get the return_if_fail or assert type treatment and not be runtime-detectable, because a nice warning is more helpful to the programmer or in bug reports, and runtime detecting these is just wrong anyway - fix the bug, don't recover from the bug by adding bug-handling code For those reasons I don't agree at all with Benjamin that GError is the "old" approach, I think it is still the right approach *for errors that should generally be handled or recover
Re: Some comments about GVFS
On Thu, 3 May 2007 11:11:39 +0200, "Benjamin Otte" wrote:
>I much prefer the cairo model, where basically the
> object keeps its own GError and has a function like cairo_status [3]
> that gives you the last error that occured.
It's worth pointing out an additional aspect of the cairo
error-handling model: The object becomes entirely inert as soon as an
error occurs within it. That is, all methods of the object first check
the status and return immediately on error.
That part of the model is essential for the application code to be
able to benefit by deferring error-checking to a convenient time.
-Carl
PS. After I wrote that much I also wrote the following rambling
treatise on the model. I didn't have time to make it shorter, so feel
free to delete it. (I almost deleted it myself, but I think someone
might find it useful).
I think the model is extremely successful in that it actually becomes
practical to write correct programs. We started with the assumption
that programmers rarely include all the necessary checks to make their
program correct, so we decided to make the program correct without all
of those checks.
And that means that the correct program can remain readable---there's
a lot less clutter without from error-checking paths. I love the fact
that code samples in the documentation of the library don't need those
often-seen disclaimers, "error-checking code removed for readability".
Another great benefit is that compiler features such as
attribute(warn_unused_result) can actually become feasible to use,
(that is, it doesn't result in a bunch of false-positive noise).
For example, in cairo's core API of about 200 functions, there are
less than 15 that can return an error status indication, (not counting
the calls where the user is explicitly asking for a status value). All
other functions are either void or are returning a value that is of
direct interest to the caller, (such as a constructor or a "get"
function). So, from the calling convention, it's obvious to the user
that the great majority of the time there's no further error-checking
required. And, in the few functions that _do_ return an error status,
it's actually very important for the user to check those, (and
something like the warn_unused_result attribute can be quite helpful
for this).
One thing that can be get trickier with this model is tracking down
what actually caused the error once it is detected. While it's
convenient to be able to defer the error checking, this also means
that detection of the error becomes separated in time and code from
when the error was committed. (With the "old" approach there is often
unintended deferral due to missing error-checks, but hopefully the
user gets lucky and a crash happens soon after the error. But the
"inert" stuff described above prevents this.)
So, to successfully adopt this model, the user really needs to be
provided with some means for getting early reports about detected
errors. Cairo is quite conservative about this, providing only a
function that can be set as a breakpoint for when an error is
detected---and that's probably not quite as much help as will be
desired in many cases. Within a glib world there should be no
compunction in spewing messages or allowing applications to register a
callback for when errors are detected by the library.
Also, another subtle issue is that application code can be incorrect
if it depends on side-effects of library calls that will not always
happen in the face of inert methods. For example, imagine some
fictitious code that looks like this:
while (collection_has_more_items (collection)) {
...
collection_remove_item (collection);
}
...
/* Finally check status of collection here. */
if (collection_has_error (collection))
handle_error();
If collection_remove_item could become inert, then the implicit
side-effect of reducing the return-value of collection_has_more_items
would be violated, and the application code would result in an
infinite loop.
So this would be a situation where the inert-object style would not
help at all. Writing a correct program in this case would be more
difficult, (the user would have to anticipate the problem and add an
extra call to check for errors within the loop), than if the
remove_item call directly returned an error indication, (letting the
user know it is important to check for that error).
For cairo, we largely get to ignore this issue, simply because the
side-effects of cairo operations, (drawing operations), rarely have
such a direct impact on a program's control flow.
So that's something else to keep in mind if considering this style.
Finally, cairo also avoids returning NULL from failed object-creation
functions. (The idea being to return a non-NULL object that can
indicate what type of failure occurred.) I don't know that that aspect
has been entirely successful. The application code ends up
