Re: oaf locking bug ...

[Maciej Stachowiak <mjs noisehavoc org>]

On 26Sep2001 11:32PM (-0400), Michael Meeks wrote:
> 
> Found it:
> 
> 	We have to open the lockfile RW to take a the lock we need, the
> fcntl was always failing silently;
> 
> 	May I commit ?

Hi Michael,

I just committed a similar fix which I did this morning. Thanks for
your analysis on this problem.

 - Maciej

> 
> 	Regards,
> 
> 		Michael.
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/oaf/ChangeLog,v
> retrieving revision 1.181
> diff -u -r1.181 ChangeLog
> --- ChangeLog	2001/09/12 23:15:51	1.181
> +++ ChangeLog	2001/09/27 03:28:31
> @@ -1,3 +1,9 @@
> +2001-09-28  Michael Meeks  <michael ximian com>
> +
> +	* liboaf/oaf-registration.c (rloc_file_lock): open a
> +	writable lock read / write or we'll can't do proper locking,
> +	add a warning on this error for the future.
> +
>  2001-09-12  Darin Adler  <darin bentspoon com>
> 
>  	* oafd/ac-corba.c: Add a missing include of config.h.
> Index: liboaf/oaf-registration.c
> ===================================================================
> RCS file: /cvs/gnome/oaf/liboaf/oaf-registration.c,v
> retrieving revision 1.31
> diff -u -r1.31 oaf-registration.c
> --- liboaf/oaf-registration.c	2001/09/04 03:46:52	1.31
> +++ liboaf/oaf-registration.c	2001/09/27 03:28:34
> @@ -450,19 +451,25 @@
>  	struct flock lock;
> 
>          fn = g_strdup_printf ("/tmp/orbit-%s/oaf-register.lock", g_get_user_name ());
> +
> +	lock_fd = open (fn, O_CREAT | O_RDWR, 0700);
> 
> -	lock_fd = open (fn, O_CREAT | O_RDONLY, 0700);
>  	fcntl (lock_fd, F_SETFD, FD_CLOEXEC);
> 
>  	if (lock_fd >= 0) {
> +                int ret;
>  		lock.l_type = F_WRLCK;
>  		lock.l_whence = SEEK_SET;
>  		lock.l_start = 0;
>  		lock.l_len = 1;
>  		lock.l_pid = getpid ();
> 
> -		while (fcntl (lock_fd, F_SETLKW, &lock) < 0
> +		while ((ret = fcntl (lock_fd, F_SETLKW, &lock)) < 0
>  		       && errno == EINTR) /**/;
> +
> +                if (ret < 0)
> +                        g_warning ("%d: locking error '%s'",
> +                                   getpid (), strerror (errno));
>  	}
> 
>          g_free (fn);
> 
> 	And:
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/gnome/bonobo-activation/ChangeLog,v
> retrieving revision 1.227
> diff -u -r1.227 ChangeLog
> --- ChangeLog	2001/09/26 07:57:59	1.227
> +++ ChangeLog	2001/09/27 03:31:08
> @@ -1,3 +1,10 @@
> +2001-09-28  Michael Meeks  <michael ximian com>
> +
> +	* bonobo-activation/bonobo-activation-base-service.c
> +	(rloc_file_lock): open file read / write - since there is no
> +	chance of EEXIST being raised since we don't use O_EXCL, need
> +	read/write to be able to F_SETLKW with a F_WRLCK
> +
>  2001-09-26  Maciej Stachowiak  <mjs noisehavoc org>
> 
>  	* NEWS, configure.in: Updated for 0.9.2.
> Index: bonobo-activation/bonobo-activation-base-service.c
> ===================================================================
> RCS file:
> /cvs/gnome/bonobo-activation/bonobo-activation/bonobo-activation-base-service.c,v
> retrieving revision 1.32
> diff -u -r1.32 bonobo-activation-base-service.c
> --- bonobo-activation/bonobo-activation-base-service.c	2001/08/01
> 01:57:02	1.32
> +++ bonobo-activation/bonobo-activation-base-service.c	2001/09/27
> 03:31:14
> @@ -456,35 +456,25 @@
> 
>          fn = g_strdup_printf ("/tmp/orbit-%s/oaf-register.lock",
> g_get_user_name ());
> 
> -	while ((lock_fd = open (fn, O_CREAT | O_RDONLY, 0700)) < 0) {
> -		if (errno == EEXIST) {
> -#ifdef HAVE_USLEEP
> -			usleep (10000);
> -#elif defined(HAVE_NANOSLEEP)
> -			{
> -				struct timespec timewait;
> -				timewait.tv_sec = 0;
> -				timewait.tv_nsec = 1000000;
> -				nanosleep (&timewait, NULL);
> -			}
> -#else
> -			sleep (1);
> -#endif
> -		} else
> -			break;
> -	}
> +        lock_fd = open (fn, O_CREAT | O_RDWR, 0700);
> 
>  	fcntl (lock_fd, F_SETFD, FD_CLOEXEC);
> 
>  	if (lock_fd >= 0) {
> +                int ret;
> +
>  		lock.l_type = F_WRLCK;
>  		lock.l_whence = SEEK_SET;
>  		lock.l_start = 0;
>  		lock.l_len = 1;
>  		lock.l_pid = getpid ();
> 
> -		while (fcntl (lock_fd, F_SETLKW, &lock) < 0
> +		while ((ret = fcntl (lock_fd, F_SETLKW, &lock)) < 0
>  		       && errno == EINTR) /**/;
> +
> +                if (ret < 0)
> +                        g_warning ("%d: locking error '%s'",
> +                                   getpid (), strerror (errno));
>  	}
> 
>          g_free (fn);
> 
> 
> -- 
>  mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot
> 
> 
> _______________________________________________
> gnome-components-list mailing list
> gnome-components-list gnome org
> http://mail.gnome.org/mailman/listinfo/gnome-components-list