[PATCH] tevent glib event loop glue

[Noel Power]

On 16/02/16 10:04, Ralph Boehme wrote:
> Hi,
>
> On Mon, Feb 15, 2016 at 04:56:21PM +0100, Ralph Boehme wrote:
>> On Sat, Feb 13, 2016 at 09:38:19AM +0100, Stefan Metzmacher wrote:
>>>> Attached is a patchset that adds support for polling a glib
>>>> g_main_context from tevent. Later commits make use of it in the mdsvc
>>>> RPC server.
>>>>
>>>> The second patch adds a test binary that can be used to run query
>>>> Gnome Tracker either using the native glib event loop or tevent.
>>>>
>>>> I'm still unsure whether we want this added directly to tevent at this
>>>> early stage, or instead better put it to source3/lib/. It makes no
>>>> difference for Samba internal consumers.
>>> I'd prefer to keep in Samba only for now (as
>>> lib/util/tevent_glib_glue.[ch]) and adding a 'samba_' prefix to the
>>> public function,so that we won't conflict, if we later also add this
>>> to tevent.
>> Updated patchset attached.
>>
>> Please review&push if ok. Thanks!
> attached is an updated patchset with a slightly modified build: the
> tevent-glib-glue subsytem is only enabled if requested by other
> components like the mdssvc RPC service that require it.
>
Well, I'd like to ask we hold off committing this (at least until the
attached patches are evaluated), I've discovered an issue that leads me
to believe that the current implementation wont work. I've come to the
conclusion that actually going back to the idea of using trace callbacks
is probably the only viable solution. The main problem with the current
solution is that if any glib event sources are modified/created/deleted
in the context of a tevent handler that isn't associated with the
integration implementation then we don't pick up any of those changes
(see first patch that modifies the client for a reproducer) This problem
can be solved by ensuring that each loop iteration does the full set of
steps e.g.  prepare, poll, check, dispatch.

The pros;
  + besides the issue of the holding the context (see below) this
solution practically satisfies all the original requirements that I had
set myself when I started with my original prototype
  + the implementation imho is simpler, particularly the implementation
of glib_process and the timeout = 0 optimisation (see patch commit
message for more details)
  + it works (but careful review needed, this stuff is tricky)
The cons;
 + glib context still held longer than I would like (but that is no
different than the current solution)
 + trace callbacks imho is not really the most intuitive api to hook
into but at least in this incarnation it is self contained and not
fragmented across other modules

Of course there maybe an alternative approach, I'd be happy to pursue
any suggested one.

Thanks,
Noel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Reorganise-glib-glue-code-use-TEVENT_TRACE_-BEFORE-A.patch
Type: text/x-diff
Size: 10565 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/8f4f00d4/0002-Reorganise-glib-glue-code-use-TEVENT_TRACE_-BEFORE-A.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Adjust-test-client-to-create-activity-in-glib-event-.patch
Type: text/x-diff
Size: 2595 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160222/8f4f00d4/0001-Adjust-test-client-to-create-activity-in-glib-event-.diff>