[PATCH] Fix terminate connection behavior for asynchronous endpoint with PUSH notification flavors
Stefan (metze) Metzmacher
metze at samba.org
Tue Apr 7 00:39:33 MDT 2015
> Find below a third version of the patch which now loops over all
> contexts to call the destructor. It is the ubind function which is
> responsible for managing/cleaning up context->conn->pending_call_list.
> I have also moved the code into dcesrv_cleanup_broken_connections()
> because this function is called first whenever there are outstanding
> pending calls.
> Does this approach sounds better/more complete?
Yep, if you know move context_cur->iface = NULL; into the destructor I'm
> On 31/03/15 19:05, Julien Kerihuel wrote:
>> Following-up on the chat we had this morning, I have updated the patch
>> The new patch explicitly calls the unbind function in
>> dcesrv_terminate_connection() if the flag (renamed to)
>> DCESRV_CALL_STATE_FLAG_PROCESS_PENDING_CALL is set.
>> The unbind function of the interface is responsible for dealing with
>> pending_calls and cleaning-up the list.
>> Does this look more like what you had in mind?
>> On 30/03/15 15:47, Julien Kerihuel wrote:
>>> On 30/03/15 13:22, Stefan (metze) Metzmacher wrote:
>>>> Now, because I call dcesrv_reply() from the callback, it means that I
>>>> have an outstanding pending call as long as I didn't receive a
>>>> notification on this custom p->fd socket from a remote system.
>>>> But how is dcerpc_server_asyncemsmdb_unbind() called, with the your changes?
>>>> I don't see it called from dcesrv_terminate_connection().
>>> This is a very good question.
>>> I had troubles tracking down how the destructor was called due to the
>>> asynchronous mechanism in place and I am afraid I still miss some bits
>>> of it. The patch is relying on these assumptions and observations:
>>> 1. when the client disconnects and there is no pending call, the
>>> dcerpc server remove dce_conn from broken connections, terminate the
>>> connection using stream_terminate_connection and
>>> dcesrv_connection_context_destructor is called. This destructor is
>>> responsible for calling the unbind function of the interface:
>>> #0 dcesrv_connection_context_destructor (c=0x7f50ffbcd0e0) at ../source4/rpc_server/dcerpc_server.c:499
>>> #1 0x00007f50f947eb3d in ?? () from /usr/lib/x86_64-linux-gnu/libtalloc.so.2
>>> #2 0x00007f50f947e87b in ?? () from /usr/lib/x86_64-linux-gnu/libtalloc.so.2
>>> #3 0x00007f50f947af33 in _talloc_free () from /usr/lib/x86_64-linux-gnu/libtalloc.so.2
>>> #4 0x00007f50ebedddf6 in dcesrv_read_fragment_done (subreq=0x0) at ../source4/rpc_server/dcerpc_server.c:1563
>>> 2. when the client disconnects and there is pending call, the dcerpc
>>> server adds the connection to dce_ctx->broken_connections but
>>> dcesrv_connection_context_destructor is never called.
>>>>> What is implemented in Samba is correct in all cases with regards to
>>>>> pending calls except this one. In this case, and without this patch,
>>>>> this connection is going to be considered broken 100% of the time.
>>>>> What I want, it for this connection to behave normally upon
>>>>> disconnection from client (close Outlook) and not be considered as a
>>>>> broken connection.
>>>> Why if the client disconnect, the connection is broken, why should we
>>>> it's not?
>>> Because if we don't, then we can't execute teardown code upon client
>>> disconnection. If the endpoint is part of a more global asynchronous
>>> infrastructure, we may have to communicate with other services to
>>> invalidate a user session or notify a user has disconnected. That is the
>>> context in which asyncemsmdb endpoint is: upon client disconnection, we
>>> notify a resolution name service and delete from the record the entry
>>> referring to the customer socket associated to the connection.
>>> Given that I don't have (without the patch) any way to invalidate the
>>> remote data from asyncemsmdb, I end up with deprecated cached session
>>> information. I could invalidate it with the client, but it is less
>>> elegant and really should be a task for the service who registered it in
>>> the first place, hence asyncemsmdb.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 198 bytes
Desc: OpenPGP digital signature
More information about the samba-technical