Patches for https://bugzilla.samba.org/show_bug.cgi?id=11182
obnox at samba.org
Wed May 6 13:29:00 MDT 2015
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.
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
Size: 198 bytes
Desc: not available
More information about the samba-technical