On Tue, Apr 26, 2005 at 03:33:11PM -0400, Yannick Bergeron wrote:
> Hi Andrew,
> 
> I know that you're a very busy guy, but I've CC you on this post a few days 
> ago and didn't get any answer.
> 
> http://lists.samba.org/archive/samba-technical/2005-April/040377.html
> http://lists.samba.org/archive/samba-technical/2005-April/040378.html
> http://lists.samba.org/archive/samba-technical/2005-April/040379.html
> http://lists.samba.org/archive/samba/2005-April/104135.html
> 
> I would like to have your impression on this problem or if you can 
> reproduce it.
> 
> I would be really surprised if I was the only one who has it. The 
> difference is that we're using samba with a lots of users and we've 
> estimated that the file will grow at least 55mb per month. This if for a 
> smbd child that make only one connection. It could be like 20 times bigger 
> (1.1gb) if each smbd child owns 20 connections.

There were some codepaths in smbd/service.c that didn't call yield_connection()
on an error. But they would only occur in an error case. Do you have any of 
these
in you smbd logs ?

Anyway, here is a patch to ensure yield_connection() is always called on error
(and also a tidyup of some extraneous macro calls).

Jeremy.
Index: smbd/service.c
===================================================================
--- smbd/service.c      (revision 6493)
+++ smbd/service.c      (working copy)
@@ -521,7 +521,7 @@
        /* Initialise VFS function pointers */
 
        if (!smbd_vfs_init(conn)) {
-               DEBUG(0, ("vfs_init failed for service %s\n", 
lp_servicename(SNUM(conn))));
+               DEBUG(0, ("vfs_init failed for service %s\n", 
lp_servicename(snum)));
                conn_free(conn);
                *status = NT_STATUS_BAD_NETWORK_NAME;
                return NULL;
@@ -544,8 +544,8 @@
 /* ROOT Activities: */ 
        /* check number of connections */
        if (!claim_connection(conn,
-                             lp_servicename(SNUM(conn)),
-                             lp_max_connections(SNUM(conn)),
+                             lp_servicename(snum),
+                             lp_max_connections(snum),
                              False,0)) {
                DEBUG(1,("too many connections - rejected\n"));
                conn_free(conn);
@@ -555,16 +555,16 @@
 
        /* Preexecs are done here as they might make the dir we are to ChDir to 
below */
        /* execute any "root preexec = " line */
-       if (*lp_rootpreexec(SNUM(conn))) {
+       if (*lp_rootpreexec(snum)) {
                int ret;
                pstring cmd;
-               pstrcpy(cmd,lp_rootpreexec(SNUM(conn)));
+               pstrcpy(cmd,lp_rootpreexec(snum));
                standard_sub_conn(conn,cmd,sizeof(cmd));
                DEBUG(5,("cmd=%s\n",cmd));
                ret = smbrun(cmd,NULL);
-               if (ret != 0 && lp_rootpreexec_close(SNUM(conn))) {
+               if (ret != 0 && lp_rootpreexec_close(snum)) {
                        DEBUG(1,("root preexec gave %d - failing connection\n", 
ret));
-                       yield_connection(conn, lp_servicename(SNUM(conn)));
+                       yield_connection(conn, lp_servicename(snum));
                        conn_free(conn);
                        *status = NT_STATUS_ACCESS_DENIED;
                        return NULL;
@@ -575,6 +575,7 @@
        if (!change_to_user(conn, conn->vuid)) {
                /* No point continuing if they fail the basic checks */
                DEBUG(0,("Can't become connected user!\n"));
+               yield_connection(conn, lp_servicename(snum));
                conn_free(conn);
                *status = NT_STATUS_LOGON_FAILURE;
                return NULL;
@@ -584,16 +585,16 @@
        
        /* Preexecs are done here as they might make the dir we are to ChDir to 
below */
        /* execute any "preexec = " line */
-       if (*lp_preexec(SNUM(conn))) {
+       if (*lp_preexec(snum)) {
                int ret;
                pstring cmd;
-               pstrcpy(cmd,lp_preexec(SNUM(conn)));
+               pstrcpy(cmd,lp_preexec(snum));
                standard_sub_conn(conn,cmd,sizeof(cmd));
                ret = smbrun(cmd,NULL);
-               if (ret != 0 && lp_preexec_close(SNUM(conn))) {
+               if (ret != 0 && lp_preexec_close(snum)) {
                        DEBUG(1,("preexec gave %d - failing connection\n", 
ret));
                        change_to_root_user();
-                       yield_connection(conn, lp_servicename(SNUM(conn)));
+                       yield_connection(conn, lp_servicename(snum));
                        conn_free(conn);
                        *status = NT_STATUS_ACCESS_DENIED;
                        return NULL;
@@ -601,7 +602,7 @@
        }
 
 #ifdef WITH_FAKE_KASERVER
-       if (lp_afs_share(SNUM(conn))) {
+       if (lp_afs_share(snum)) {
                afs_login(conn);
        }
 #endif
@@ -612,9 +613,9 @@
           I have disabled this chdir check (tridge) */
        /* the alternative is just to check the directory exists */
        if (SMB_VFS_STAT(conn, conn->connectpath, &st) != 0 || 
!S_ISDIR(st.st_mode)) {
-               DEBUG(0,("'%s' does not exist or is not a directory, when 
connecting to [%s]\n", conn->connectpath, lp_servicename(SNUM(conn))));
+               DEBUG(0,("'%s' does not exist or is not a directory, when 
connecting to [%s]\n", conn->connectpath, lp_servicename(snum)));
                change_to_root_user();
-               yield_connection(conn, lp_servicename(SNUM(conn)));
+               yield_connection(conn, lp_servicename(snum));
                conn_free(conn);
                *status = NT_STATUS_BAD_NETWORK_NAME;
                return NULL;
@@ -642,7 +643,7 @@
        if( DEBUGLVL( IS_IPC(conn) ? 3 : 1 ) ) {
                dbgtext( "%s (%s) ", get_remote_machine_name(), 
conn->client_address );
                dbgtext( "%s", srv_is_signing_active() ? "signed " : "");
-               dbgtext( "connect to service %s ", lp_servicename(SNUM(conn)) );
+               dbgtext( "connect to service %s ", lp_servicename(snum) );
                dbgtext( "initially as user %s ", user );
                dbgtext( "(uid=%d, gid=%d) ", (int)geteuid(), (int)getegid() );
                dbgtext( "(pid %d)\n", (int)sys_getpid() );
@@ -650,9 +651,9 @@
        
        /* Add veto/hide lists */
        if (!IS_IPC(conn) && !IS_PRINT(conn)) {
-               set_namearray( &conn->veto_list, lp_veto_files(SNUM(conn)));
-               set_namearray( &conn->hide_list, lp_hide_files(SNUM(conn)));
-               set_namearray( &conn->veto_oplock_list, 
lp_veto_oplocks(SNUM(conn)));
+               set_namearray( &conn->veto_list, lp_veto_files(snum));
+               set_namearray( &conn->hide_list, lp_hide_files(snum));
+               set_namearray( &conn->veto_oplock_list, lp_veto_oplocks(snum));
        }
        
        /* Invoke VFS make connection hook */
@@ -660,6 +661,7 @@
        if (SMB_VFS_CONNECT(conn, lp_servicename(snum), user) < 0) {
                DEBUG(0,("make_connection: VFS make connection failed!\n"));
                change_to_root_user();
+               yield_connection(conn, lp_servicename(snum));
                conn_free(conn);
                *status = NT_STATUS_UNSUCCESSFUL;
                return NULL;
@@ -812,8 +814,9 @@
 }
 
 /****************************************************************************
-close a cnum
+ Close a cnum.
 ****************************************************************************/
+
 void close_cnum(connection_struct *conn, uint16 vuid)
 {
        if (IS_IPC(conn)) {
-- 
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/listinfo/samba

Reply via email to