[PATCH] s3:events: Call all ready fd event handlers on eachiteration of the main loop

Steven Danneman steven.danneman at isilon.com
Fri Sep 17 01:30:57 MDT 2010


> On Thu, Sep 16, 2010 at 02:10:47PM -0700, Steven Danneman wrote:
> > Taking the design considerations you mentioned I've reworked my
> patch.
> > It is not the Cadillac solution, as it only handles the smbd/select
> > case, but I went for minimal code disruption.
> >
> > This patch effectively changes the select loop in smbd child process
> > only, to safely iterate through all fds returned by select each
time,
> > while still handling timed and signal events round-robin.
> >
> > This passes make test and I'm throwing more intensive work loads at
> it
> > now to test it's stability.
> >
> > I've also opened bug 7686 to track this issue.  Please tell me what
> you
> > think.
> 
> I think that actually does it. The most important piece is
> that we start walking the fde list new after each handler
> was called. The crash that was fixed was due to one socket
> event triggering a TALLOC_FREE on a different fde. It was in
> winbind: accept() on the listener socket triggered an idle
> client to be thrown out, thus its fde would vanish. (iirc...)
> This meant that the linked list had changed under our feet.
> 
> While I have not tested it, I think that it can go in. A
> heavily loaded winbind will probably pretty quickly show
> this problem again. If you push it, can you also push a
> revert of d5cf6482ed0cd, so that it will not confuse
> anybody?

Yes, freeing fdes while iterating the linked list was definitely a
problem with my first proposal.

I just ran into an issue of clients disconnecting while stress testing
this patch from 10 WinXP clients with various file system operations:

2010-09-16T17:56:45-07:00 <30.3> sd-6-0-58-1(id1) smbd[3232]:
[2010/09/16 17:56:45.224699,  0, effective(65534, 65534), real(0, 0)]
smbd/nttrans.c:2052(call_nt_transact_ioctl)
2010-09-16T17:56:45-07:00 <30.3> sd-6-0-58-1(id1) smbd[3232]:
call_nt_transact_ioctl(0x900eb): Currently not implemented.
2010-09-16T17:56:46-07:00 <30.3> sd-6-0-58-1(id1) smbd[3232]:
[2010/09/16 17:56:46.649916,  0, effective(65534, 65534), real(0, 0)]
smbd/process.c:117(valid_packet_size)
2010-09-16T17:56:46-07:00 <30.3> sd-6-0-58-1(id1) smbd[3232]:   Invalid
packet length! (366712 bytes).
2010-09-16T17:56:46-07:00 <30.4> sd-6-0-58-1(id1) smbd[3232]:
[2010/09/16 17:56:46.673276,  1, effective(0, 0), real(0, 0)]
smbd/service.c:1257(close_cnum)
2010-09-16T17:56:46-07:00 <30.4> sd-6-0-58-1(id1) smbd[3232]:
10.54.15.186 (10.54.15.186) closed connection to service ifs

I need to figure out what's going on here, then I'll push the patch and
revert.

Thanks for the quick review.

-Steven


More information about the samba-technical mailing list