[PATCH] fix issues with the standard model control pipe.
Gary Lockyer
gary at catalyst.net.nz
Mon Sep 18 03:18:02 UTC 2017
Raplh,
Looks like I got the commit comment totally wrong. The intent was
to remove the global 'child_pipe'from standard_model. And have a single
fd that could be passed to the individual process models, in preparation
for the adding of the pre-fork model. Which is what this patch actually
does.
As you can guess from the debugging comment that proved to be interesting.
I will:
- reword the commit message
- only tfork a new process if there is no pthread_atfork() support
Thanks
Gary
On 18/09/17 11:23, Ralph Böhme wrote:
> On Thu, Sep 14, 2017 at 05:31:54AM +0000, Gary Lockyer via samba-technical wrote:
>> Fix issues with the ownership of the writable end of the control pipe in
>> the standard process model. Which resulted in child processes not
>> detecting that the samba daemon master process had not terminated.
>
> afaict this is not a problem in the standard process model, is it? There we
> correctly close the writable end of the pipe directly after forking the task
> services. When I look at the open pipe fds with lsof I only see it open in the
> main samba process:
>
> 6959 ? Ss 0:00 samba: root process
> 6960 ? S 0:00 \_ samba: task[s3fs_parent]
> 6962 ? S 0:00 | \_ samba: tfork waiter process
> 6964 ? Ss 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 6983 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 6984 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 6985 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 6961 ? S 0:00 \_ samba: task[dcesrv]
> 6963 ? S 0:00 \_ samba: task[nbtd]
> 6965 ? S 0:00 \_ samba: task wrepl server_id[6965]
> 6966 ? S 0:00 \_ samba: task[ldapsrv]
> 6967 ? S 0:00 \_ samba: task[cldapd]
> 6968 ? S 0:00 \_ samba: task[kdc]
> 6969 ? S 0:00 \_ samba: task[dreplsrv]
> 6970 ? S 0:00 \_ samba: task[winbindd_parent]
> 6974 ? S 0:00 | \_ samba: tfork waiter process
> 6976 ? Ss 0:00 | \_ /home/slow/git/samba/scratch/bin/winbindd -D --option=server role check:inhibit=yes --foreground
> 6986 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/winbindd -D --option=server role check:inhibit=yes --foreground
> 6971 ? S 0:00 \_ samba: task[ntp_signd]
> 6972 ? S 0:00 \_ samba: task[kccsrv]
> 6973 ? S 0:00 \_ samba: task[dnsupdate]
> 6977 ? S 0:00 \_ samba: task[dns]
>
> A debug line told me that the pipe fd in 6959 is fd 25:
>
> standard_model_init: pid [6959] pipe fd [25]
>
> $ sudo lsof -p 6959 | grep FIFO | grep 25w
> samba 6959 root 25w FIFO 0,8 0t0 86037 pipe
>
> $ sudo lsof | grep FIFO | grep samba | grep 86037
> samba 6959 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6959 root 25w FIFO 0,8 0t0 86037 pipe
> samba 6960 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6961 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6963 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6965 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6966 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6967 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6968 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6969 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6970 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6971 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6972 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6973 root 24r FIFO 0,8 0t0 86037 pipe
> samba 6977 root 24r FIFO 0,8 0t0 86037 pipe
>
> So generally, only with process model single we can end up inheriting the
> writable pipe fd, correct?
>
> Can we avoid the call to tfork() by using pthread_atfork() if available and only
> falling back to tfork() if not? Calling tfork() makes for a hard to understand
> process hierarchy, so we may want to avoid it if possible:
>
> 9310 ? Ss 0:00 samba: root process
> 9311 ? S 0:00 \_ samba: tfork waiter process
> 9312 ? S 0:00 | \_ samba: tfork waiter process
> 9313 ? S 0:00 \_ samba: task[s3fs_parent]
> 9315 ? S 0:00 | \_ samba: tfork waiter process
> 9317 ? Ss 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 9337 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 9338 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 9339 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/smbd -D --option=server role check:inhibit=yes --foreground
> 9314 ? S 0:00 \_ samba: task[dcesrv]
> 9316 ? S 0:00 \_ samba: task[nbtd]
> 9318 ? S 0:00 \_ samba: task wrepl server_id[9318]
> 9319 ? S 0:01 \_ samba: task[ldapsrv]
> 9320 ? S 0:00 \_ samba: task[cldapd]
> 9321 ? S 0:00 \_ samba: task[kdc]
> 9322 ? S 0:00 \_ samba: task[dreplsrv]
> 9323 ? S 0:00 \_ samba: task[winbindd_parent]
> 9326 ? S 0:00 | \_ samba: tfork waiter process
> 9329 ? Ss 0:00 | \_ /home/slow/git/samba/scratch/bin/winbindd -D --option=server role check:inhibit=yes --foreground
> 9336 ? S 0:00 | \_ /home/slow/git/samba/scratch/bin/winbindd -D --option=server role check:inhibit=yes --foreground
> 9324 ? S 0:00 \_ samba: task[ntp_signd]
> 9325 ? S 0:00 \_ samba: task[kccsrv]
> 9327 ? S 0:00 \_ samba: task[dnsupdate]
> 9328 ? S 0:00 \_ samba: task[dns]
>
> Processes 9311 and 9312 are introduced by the addtional tfork() and could be
> avoided if we have pthread support. Also, for reasons I don't understand yet,
> all the task processes not children of 9312 but 9310.
>
> Cheerio!
> -slow
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170918/7f82fc6e/signature.sig>
More information about the samba-technical
mailing list