GNOME Bugzilla – Bug 359979
Change e_msgport_wait() semantics
Last modified: 2007-02-07 22:00:51 UTC
I propose changing the semantics of e_msgport_wait() slightly. The function is currently implemented as a "block-and-peek" operation, but I think it makes more sense as a "block-and-pop" operation. In other words, e_msgport_wait() should actually pop the new message off the EMsgPort's internal queue and return it to the caller, rather than leaving the message on the queue and letting the caller peek at it. Rationale: - Almost all uses of e_msgport_wait() disregard the return value and immediately call e_msgport_get() to actually pop the message off the EMsgPort's internal queue [1]. The few cases that don't follow this pattern are still easy to adapt to the semantics I'm proposing. - While part of libedataserver's public API, I can't find any uses of EMsgPort outside of Evolution. - The behavior of the function is undocumented anyway. - It would simplify my new EMsgPort implementation in bug #348888. [1] Actually I'm seeing many cases of e_msgport_get() being called as part of an assertion. This in itself is a bug. Assertions should never cause side-effects, since the assertion may not execute depending on how the how the program was compiled. (see: G_DISABLE_ASSERT)
Created attachment 74139 [details] [review] Proposed patch for Evolution
Created attachment 74140 [details] [review] Proposed patch for Evolution-Data-Server Note, this patch does not modify e_msgport_wait(), just the uses of it.
Created attachment 74144 [details] Proposed EMsgPort implementation from bug #348888 I'm not sure of the best way to illustrate the EMsgPort changes, since it's based on another patch on mine that's waiting for approval. So instead of a patch, let me try this: Here's the full EMsgPort implementation that I proposed in bug #348888.
Created attachment 74147 [details] EMsgPort modifications And here's the EMsgPort modifications for this bug.
Created attachment 74833 [details] [review] Proposed patch for Evolution-Data-Server Now that the patch for bug #348888 has been committed, here's an updated patch for Evolution-Data-Server that combines attachment #74140 [details] in comment #2 with the EMsgPort modifications I'm proposing. As you can see, I'm just ripping out complexity in EMsgPort. Also, don't forget to review attachment #74139 [details] in comment #1.
See also bug #362638, which aims to kill off EThread. It makes part of the EDS patch obsolete.
Alex Larsson found a flaw in my original re-implementation of EMsgPort for bug #348888 which causes Evolution to sometimes hang after clicking "Send/Receive". The bug is due to the caching behavior I introduced in e_msgport_wait(), which I've since found to be unnecessary given the way Evolution uses the function (see comment #0). He provided a patch in bug #384183 that extends the caching mechanism in a way that maintains semantic API compatibility. Alex's patch is suitable for the stable 2.8 branch, but I'd prefer eliminating the caching mechamism altogether as a permanent solution. I'd like to see this get into 2.10 if possible. Raising the severity and priority and CC'ing harish. Can someone please review this?
Patches looks good.. please commit them to SVN Head.
Committed to HEAD.