Re: [Evolution-hackers] Camel Manifesto (update)
On Sun, 2011-03-13 at 21:32 +, David Woodhouse wrote: > Ug, and now I've found that that workaround doesn't work either. If we > try to rename a folder, we end up blocking in the main thread, waiting > for the soup request that we deliberately put into an idle function so > that it could run in the main thread... > > Thread 1 (Thread 0x77d93980 (LWP 9874)): > #0 pthread_cond_wait@@GLIBC_2.3.2 () at > ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 > #1 0x0034ff00ee2d in e_flag_wait (flag=0x2002220) at e-flag.c:130 > #2 0x7fffe387e934 in ews_get_folder_info_sync (store=0x6e42a0 > [CamelEwsStore], top=0x1480880 "asd", flags=1, error=0x0) at > camel-ews-store.c:500 > #3 0x0035020508ae in camel_store_get_folder_info (store=0x6e42a0 > [CamelEwsStore], top=0x1480880 "asd", flags=1, error=0x0) at > camel-store.c:1122 > #4 0x003505460ce8 in folder_tree_cell_edited_cb (folder_tree= optimized out>, path_string=, new_name=0x1ca6460 "asd") > at em-folder-tree.c:624 > > As before, I would prefer to *fix* the broken locking, so that we can > call the soup functions from any thread. But if I'm still not allowed to > do that, what's the best way to fix this deadlock? We talked about this on IRC already, but for the record EMFolderTree is at fault here for calling a blocking Camel function. folder_tree_cell_edited_cb() instead needs to fetch folder info asynchronously and then do the actual folder rename asynchronously. I've already peppered the function with FIXME comments to this effect. I'll try to follow up on that this week before the code freeze. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Manifesto (update)
On Thu, 2011-02-17 at 18:37 +, David Woodhouse wrote: > I was told today that the GIO (and libsoup) async methods may not be > called from a thread other than the one running the mainloop. I found a > stupid race in libsoup¹ and filed a fix, but the bug was closed INVALID > because apparently it's not *supposed* to be thread-safe. > > That's a serious issue for using it from Evolution; it means we have to > jump through stupid hoops like using idle callbacks to call it from the > main thread, just as we do for gconf. Ug, and now I've found that that workaround doesn't work either. If we try to rename a folder, we end up blocking in the main thread, waiting for the soup request that we deliberately put into an idle function so that it could run in the main thread... Thread 1 (Thread 0x77d93980 (LWP 9874)): #0 pthread_cond_wait@@GLIBC_2.3.2 () at ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:162 #1 0x0034ff00ee2d in e_flag_wait (flag=0x2002220) at e-flag.c:130 #2 0x7fffe387e934 in ews_get_folder_info_sync (store=0x6e42a0 [CamelEwsStore], top=0x1480880 "asd", flags=1, error=0x0) at camel-ews-store.c:500 #3 0x0035020508ae in camel_store_get_folder_info (store=0x6e42a0 [CamelEwsStore], top=0x1480880 "asd", flags=1, error=0x0) at camel-store.c:1122 #4 0x003505460ce8 in folder_tree_cell_edited_cb (folder_tree=, path_string=, new_name=0x1ca6460 "asd") at em-folder-tree.c:624 As before, I would prefer to *fix* the broken locking, so that we can call the soup functions from any thread. But if I'm still not allowed to do that, what's the best way to fix this deadlock? -- dwmw2 ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Manifesto (update)
On Thu, 2011-02-17 at 18:37 +, David Woodhouse wrote: > I assume that your intention is that the Camel async methods would *not* > be similarly broken, and that you should be able to call them from *any* > thread and expect them not to break? > > If so, we need be *very* careful about calling into gio. No, that's not my intention. My intention is that asynchronous functions should only ever be called from the main loop thread. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Manifesto (update)
On Mon, 2010-09-27 at 12:52 -0400, Matthew Barnes wrote: > 3. Asynchronous Methods > --- > > All the upgrades I've been making to Camel over the past year, and all > the changes described above were in preparation for this: Camel now has > an asynchronous public API. > > Let me immediately point out that there is ZERO IMPACT TO EXISTING CAMEL > PROVIDERS beyond the API changes I mentioned above. Camel providers > need not do anything special to support the new asynchronous methods. > As in GIO, all async methods default to simply calling the corresponding > synchronous method from a thread pool, which is all Evolution does. I was told today that the GIO (and libsoup) async methods may not be called from a thread other than the one running the mainloop. I found a stupid race in libsoup¹ and filed a fix, but the bug was closed INVALID because apparently it's not *supposed* to be thread-safe. That's a serious issue for using it from Evolution; it means we have to jump through stupid hoops like using idle callbacks to call it from the main thread, just as we do for gconf. Apparently, gio suffers this brain-damage too. I assume that your intention is that the Camel async methods would *not* be similarly broken, and that you should be able to call them from *any* thread and expect them not to break? If so, we need be *very* careful about calling into gio. I think perhaps we need to reset expectations about what a sane API looks like in the 21st century. We are all set to stop using gconf, print out its source code, and ritually urinate on it and then burn it. In part because it's not thread-safe and we have to do stupid things to ensure we call into it from the main thread. And yet people are still implementing new APIs with the *same* problem... -- dwmw2 ¹ https://bugzilla.gnome.org/show_bug.cgi?id=642573 ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Manifesto (update)
On Mon, 2010-09-27 at 12:52 -0400, Matthew Barnes wrote: > Transient operations are now implicit: if you push a new status > message onto a non-empty message stack, the message is treated as > transient. A transient message just means there's a longer delay > before the message is shown in Evolution's status bar, since transient > operations tend to come and go quickly. If the transient operation > finishes before the timer expires, the transient message is never > shown at all. This helps filter out messages that would otherwise > flash by too quickly for users to read. Hi, I do not like this change. Its result is that I do not see which folder is getting updated and what percentage is done on it, which I consider as one of the most valuable information being on the status bar. I only see there "Checking for new mail" now, and I have no idea about actual progress of the operation behind it. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Camel Manifesto (update)
Here's another update on how the Camel API upgrades are coming along. My initial roadmap from 2009 is referenced in [1], and my last update was in May [2]. This next round of API changes will be fairly disruptive, so I'd like to coordinate with anyone finishing up their own Camel branch so my changes don't stomp on your work. I know Chen is finishing his Maildir branch, so unless I hear from someone else I'll plan to commit as soon as Chen's changes are in. The changes are substantial, but the result is pretty straight-forward and boring. Most of these changes were presented as future goals in my May update [2]. There's three major changes: 1. Redefine CamelOperation -- I'm removing the whole thread-private CamelOperation concept, along with the register/unregister functions. Thread-private CamelOperations seem convenient but actually have a detrimental effect on the public API and is one of the major problems I have with Camel: you can't tell from the API which functions block and which ones don't. As a result, Evolution is still rife with Camel calls that block the main loop and hang the UI. The forthcoming changes will finally solve this design flaw. As in GIO, all blocking functions will take a GCancellable pointer. Since CamelOperation is now a subclass of GCancellable, any GCancellable pointer in Camel may be a CamelOperation, a plain GCancellable, or NULL. I've made some further simplifications to CamelOperation both internally and externally. The internal message stack is simpler now, and as such camel_operation_start() and camel_operation_end() have been renamed to camel_operation_push_message() and camel_operation_pop_message(), which I think are more intuitive. Transient operations are now implicit: if you push a new status message onto a non-empty message stack, the message is treated as transient. A transient message just means there's a longer delay before the message is shown in Evolution's status bar, since transient operations tend to come and go quickly. If the transient operation finishes before the timer expires, the transient message is never shown at all. This helps filter out messages that would otherwise flash by too quickly for users to read. Finally, passing NULL to any CamelOperation function is now a no-op. 2. Rename Blocking Methods -- Almost[*] all blocking method names have grown a "_sync" suffix. This has a couple advantages: - It's pretty damn obvious now when you're making a blocking Camel call from Evolution or some other client program. - Blocking Camel calls are now pretty easy to grep for (camel_.*_sync). We can finally track down those blocking the main loop and file bugs. Methods already using the word "sync" I've expanded to "synchronize" so the name isn't too weird. So for example, camel_folder_sync() is now camel_folder_synchronize_sync(). [*] I didn't bother renaming CamelStream methods, since I plan to replace then with GIO streams in the near future and clients generally don't call stream methods directly anyway. 3. Asynchronous Methods --- All the upgrades I've been making to Camel over the past year, and all the changes described above were in preparation for this: Camel now has an asynchronous public API. Let me immediately point out that there is ZERO IMPACT TO EXISTING CAMEL PROVIDERS beyond the API changes I mentioned above. Camel providers need not do anything special to support the new asynchronous methods. As in GIO, all async methods default to simply calling the corresponding synchronous method from a thread pool, which is all Evolution does. The asynchronous methods will take the old blocking method names in order to emphasize to clients that this is now the recommended way to use Camel. (In case it wasn't already obvious, no I don't care about Camel's backward-compatibility right now.) Here's a before and after example to help make this all concrete. Before: gboolean camel_folder_append_message(CamelFolder *folder, CamelMimeMessage *message, const CamelMessageInfo *info, gchar **appended_uid, GError **error) After: void camel_folder_append_message(CamelFolder *folder, CamelMimeMessage *message, CamelMessageInfo *info, gint io_priority, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) gboolean camel_folder_append_message_finish (CamelFolder *folder, GAsyncResult *result, gchar **appended_ui
Re: [Evolution-hackers] Camel Manifesto (update)
I wanted to give a status update on how this is progressing, since some of you may have noticed me breaking Camel's API with gleeful abandon since the 2.31 development cycle began. My initial roadmap is here: http://mail.gnome.org/archives/evolution-hackers/2009-November/msg00019.html What's Accomplished --- - Only may be included from outside of Camel's core. - The API is slowly being sealed up, with public instance data being moved to private structs, along with new accessor functions and in some cases GObject properties. I'm working on this incrementally. - CamelObject is now a GObject subclass, and Camel's old type system is completely gone. - The CamelArg API has been replaced with GObject properties. Persistent properties (whose values are saved to the object's binary state file) are implemented by extending GParamFlags with a new flag (CAMEL_PARAM_PERSISTENT) which you can specify when registering the GObject property. camel_object_state_read/write() query the object class for properties with this flag. We currently only use state files to save a handful of booleans, so I've dropped support for other types until we actually need more. In the distant future I may kill off binary state files completely and delegate the job to EShellView's "state" key file. - CamelObject events have been replaced by GObject signals. Short Term Goals - Replace CamelException with GError. I already have this mostly done on a branch. It's straight-forward busywork except for one issue: Poor error handling often leads to multiple errors piling up on a single exception instance. I consider this a programming error regardless of how the exception mechanism handles it. GError handles this by accepting the first error and rejecting subsequent attempts to set an error with a run-time warning (unless the GError is explicitly cleared). CamelException just silently overwrites previous errors with new errors. In the coming days I'll be committing a patch to make CamelException behave like GError in this respect. I'm hoping this will expose some of more egregious error handling issues in Camel providers, but the truth is we'll probably be hunting these down for some time. Once things are stable under the new CamelException semantics, the GError transition should proceed with minimal disruption. - I would like for all blocking methods in Camel to explicitly take a GCancellable or CamelOperation argument, similar to GIO. (Turning CamelOperation into a GCancellable subclass seems like a good fit.) This will happen on a branch for two reasons: 1) not exactly sure how to get there from here yet, so I'll need to experiment a bit, and 2) I may be blocked on the TLS support for GIO since SSL TCP streams need a secure file descriptor to support cancellations, which GCancellable does not yet offer. Long-Term Goals --- - All blocking methods in Camel will have corresponding "async" and "finish" methods, whose default implementation will spawn a thread, run the synchronous method, and pass the result to the user-supplied GAsyncReadyCallback in the main thread. This closely follows the asynchronous pattern in GIO. This approach avoids the need to rewrite all Camel providers to be asynchronous, but they will be free to override the async methods if they so choose. There may be cases where it's more natural to do an operation asynchronously than synchronously. - Once all of this is in place and TLS support has landed in GIO, GIO streams will replace CamelStreams. - In the far distant future I'd like to convert all the MIME filters to the new GConverter data conversion interface for GIO streams, and maybe offer up a few of them to GIO itself. The short-term goals are a priority for 3.0, the long-term goals are just stuff I'm planning for some future Evolution 3.x release. Matthew Barnes ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
