Patches for https://bugzilla.samba.org/show_bug.cgi?id=11182

[Michael Adam]

On 2015-05-06 at 12:15 -0700, Jeremy Allison wrote:
> On Wed, May 06, 2015 at 08:53:38PM +0200, Michael Adam wrote:
> > 
> > Now I feel kind of stupid having spent my afternoon
> > reviewing the patches... ;) When I thought I was done,
> > Ira brought up one potential issue, the discussion of
> > which kept me busy until dinner time, which is why I
> > can only follow up now:
> 
> Don't feel bad. Metze specifically asked me to
> look at it, as I was the one who created the
> original tag_state_session_ptr() horror :-).

It's all well. I just needed to say it. ;)

> > In the session_shutdown helpers mentioned above,
> > compounded requests are excluded from sending
> > cancels. Ira points out that when there is a
> > compounded request in progress, the last of which
> > is a notify, and we are just at one of the earlier
> > requests when the shutdown_send is called, isn't
> > there a chance that we block indefinitely waiting
> > for the notify to vanish from the queue since it
> > in the compound and we don't send a cancel?
> 
> Let me look closely at this. Sounds right,
> but I need to study the code more.

Right.

Metze's argument (I briefly discussed with him) is that
it was this way before. And since only the last in a
compound (except for oplock breaking create) can go async
and is taken out of the compound then, we are not in danger.

But the concern is this: doesn't the introduction of the
Queue introduce a new potential for blocking (by prevent
the session from being freed waiting for the notify to disappear)?
(Which is arguably still better than crashing because
of use after free... :)

> > Attached find my reviewed version of the patchset:
> > - The session_shutdown helper is flagged with a Q(...)
> > - I have added some refactoring of an obvious code
> >   duplication added by the patches.
> >   (the removal of the code duplication is still there
> >    as separate patches for squashing with metze's original patches)
> > - I have done minor changes to some commit messages:
> >   - few typo fixes
> >   - added explanation of the session_shutdown helpers
> >   - added bug references where missing
> 
> If my autobuild goes through just add your
> fixes on top !

Ah it's in AB already - ok.
Will propose the refactoring again on top.
The commit msg fixes are not that important.

Cheers - Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150506/bcd400b8/attachment.pgp>