On Fri, Jun 27, 2014 at 01:50:39PM +0200, u-codalist-rcma_at_aetey.se wrote:
> This bug/leak becomes noticeable (and pretty harmful) when you make
> frequent updates of the databases and have a long uptime.
> 
> The patch below is not suggested as-is, rather as an illustration.
> It is though syntactically and semantically correct AFAICT.
> 
> It looks like the "checking" code was written with server notification
> via a signal in mind and then lost the corresponding fclose() when the
> signal sending part was replaced with an RPC.

Nice find, this has been broken since at least the very first CVS
checking of the updateclnt.cc code in 1996. But that version does
include the #ifdef'd out code fragment where the handle was supposed to
be closed,

    #ifdef notdef
            int pid;
            fscanf(file, "%d", &pid);
            fclose(file);
            int fd = open("/vice/srv/DATABASE", O_RDWR + O_CREAT, 0666);
            close(fd);
            kill(pid, SIGXCPU);
            LogMsg(0, SrvDebugLevel, stdout, "Notify PID %d that db changed\n", pid);
    #endif notdef

This code was then removed in a cleanup sweep about a year later.

> ----
> --- coda-src/update/updateclnt.cc.ori   2014-06-27 12:38:19.271390517 +0200
> +++ coda-src/update/updateclnt.cc       2014-06-27 12:48:03.034448819 +0200
> @@ -245,6 +245,8 @@
>                 if (nservers != 1)
>                     vice_dir_init (vicedir, i+1);
>                 /* signal file server to check data bases */
> +/* here we badly leaked open file descriptors without any reason -- rl */
> +#if 0 /* this check is irrelevant */
>                 file = fopen(vice_file("srv/pid"), "r");
>                 if (file == NULL) {
>                     LogMsg(0, SrvDebugLevel, stdout,
> @@ -252,6 +254,9 @@
>                            vice_file("srv/pid"),
>                            ViceErrorMsg(errno));
>                 } else {
> +#else /* just go ahead and try binding */
> +               {
> +#endif
>                     RPC2_Handle rpcid;
>                     if (U_BindToServer(hostlist[i], &rpcid) == RPC2_SUCCESS) {
>                         if (VolUpdateDB(rpcid) == RPC2_SUCCESS) {
> ----

I'll probably end up changing your patch to just remove all of the code
that tries to open the file instead of leaving the broken code hanging
around in an #if 0.

Jan