[PATCH] tevent-glib-glue

[Noel Power]

Fix a compile error with the fixup patch

On 22/03/2019 11:08, Noel Power wrote:
> Hi Ralf,
>
> A little later than I expected. So still I haven't looked at the mdssvc
> parts. About the glue code at least I had a pretty good look.
>
> + in 'tevent_glib_glue_trace_callback' we currently are calling
> tevent_glib_prepare potentially every trace point so possibly 4 times
> per tevent loop iteration (depending on what fires)
>
> + 'g_main_context_acquire' & 'g_main_context_release'  calls need to be
> matched (currently they are not) otherwise the gmain_ctx owner_counter
> will runaway making it impossible for a different thread acquire the context
>
> note: from glib doc
>
>     "g_main_context_release ()
>
>        void g_main_context_release (GMainContext *context);
>
>     Releases ownership of a context previously acquired by this thread
> with g_main_context_acquire(). If the context was acquired multiple
> times, the ownership will be released only when g_main_context_release()
> is called as many times as it was acquired."
>
> I think similar handling to skip_glue_trace_event I had and using just
> TEVENT_TRACE_AFTER_LOOP_ONCE with tevent_glib_process (instead of
> tevent_glib_prepare) should ensure the correct ordering of
> acquire/release (see attached patch to your which simplifies my previous
> approach with this and the other issues mentioned here and below)
>
> + After calling tevent_glib_glue_cleanup we need to re-initialiase/re-alloc
>   some glue context items to avoid using freed memory
>
> + Similarly we need to initialise some additional glue context members,
> num_maps
>   definitely needs to be reset to 0 (and probably gtimeout, gpriority etc_
>
> I haven't had a chance to run your latest patches (or my adjustments to
> those patches) on WSP yet (I will try to do that today)
>
> I tried to run make test TESTS=samba3.tevent_glib_glue with no luck,
> doesn't seem to think the test exists (I don't know what I did wrong :/)
> Note: I can run the test manually from test_tevent_glib_glue.sh
>
> Also I noticed that the tests should run regardless of whether glib_glue
> stuff is built (via fake tevent_glib_glue_test generated in the test) I
> don't love that approach as it looks like the glue test has run and
> passed, I mean the current CI looks like it is running with the
> glib_glue stuff but you need --enable-spotlight for the glib_glue stuff
> to be built and afaics none of the CI (or autobuild) test jobs enable that.
>
> I'll try to get to the main mdssvc patches after I try and test with WSP
> again
>
> thanks,
> Noel
>
> On 20/03/2019 10:14, Noel Power wrote:
>> On 18/03/2019 10:55, Ralph Böhme wrote:
>>> Hi Noel, sorry for the delay, had two days off. 
>> No problem, I am just back today from even more days than that off
>> :->>> I see below (snipped) you have a new patch set, I'll get to it
>> asap, just beware the usual post days off mail mountain will interfere
>> a bit :-)
>>> On Thu, Mar 14, 2019 at 05:15:07PM +0000, Noel Power wrote: 
>> [...]
>>>> The problem I see is that if this glib tracker api results in any
>>>> changes to the set of glib fd(s) or timeouts that need to be
>>>> monitored then the glue code may not be aware of these changes
>>>> (because the prepare phase is only called from glib glue tevent
>>>> handlers) See attached async_tracker.c patch which attempts to fake
>>>> these scenarios. [1] 
>>> ah, finally got it. :) This should be addressed in the attached
>>> patchset. I'm basically taking you tevent tracepoint approach with
>>> some simplifications. I've also added tests to the testsuite that
>>> cover both cases: adding a new glib event source from a glib event
>>> handler as well as adding one from a tevent event handler. Updated
>>> patchset attached. CI:
>>> https://gitlab.com/samba-team/devel/samba/pipelines/52336817 -slow 
>> cool, will start to get into this hopefully end of today, thanks alot
>> Noel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Some-review-fixups.patch
Type: text/x-patch
Size: 2798 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190322/a340d93f/0001-Some-review-fixups.bin>