VFS connection data cleanup with multiple modules

[Constantine Vetoshev]

On Tue, Sep 30, 2008 at 4:54 PM, Jeremy Allison <jra at samba.org> wrote:
>> When I disconnect from the share, I expect that release_data_1 and
>> release_data_2 both run, but logging shows that only release_data_1
>> runs. I can force release_data_2 to run by putting an explicit call to
>> it from a disconnect callback for vfs_object_2, but that doesn't seem
>> right.
>>
>> Am I missing something about how the release functions should work?
>
> It should get called from conn_free_internal() when the
> connection struct is removed. See this code :
>
>        /* Free vfs_connection_struct */
>        handle = conn->vfs_handles;
>        while(handle) {
>                DLIST_REMOVE(conn->vfs_handles, handle);
>                thandle = handle->next;
>                if (handle->free_data)
>                        handle->free_data(&handle->data);
>                handle = thandle;
>        }
>
> which should call all the handle release functions in
> reverse order of registration.

After poking at the code with gdb, I think I see a minor bug here.
Before the loop enters, conn->vfs_handles looks good, and
conn->vfs_handles->next also looks correct (as do all subsequent
->next pointers). Then, in the first iteration, after DLIST_REMOVE,
handle->next is NULL, so thandle is set to NULL. Looking at the
implementation of DLIST_REMOVE, I understand why: the p argument sets
its next and prev pointers to NULL if list != p, which it definitely
is not after list gets assigned to p->next. The code then sets handle
to thandle (which is NULL), and the loop terminates after checking
handle. In effect, free_data gets called on just the first handle.

A trivial workaround is to just set thandle before calling DLIST_REMOVE.

This code hasn't changed from 3.0.28a and 3.2.4, so the problem should
affect 3.2.4.