[GRASS-dev] [GRASS GIS] #755: trying to start in mapset you don't own gives wrong error msg

#755: trying to start in mapset you don't own gives wrong error msg
---------------------------+------------------------------------------------
Reporter: hamish | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Keywords: mapset access | Platform: Linux
      Cpu: x86-32 |
---------------------------+------------------------------------------------
Hi,

if I do "chmod 999.999 $LOCATION/user1" to make my user account not the
owner of a mapset, and then start grass with the full path to the mapset I
get this error:

{{{
Cleaning up temporary files ...
Starting GRASS ...
$LOCATION/user1/.gislock: Permission denied
ERROR: $GISBASE/etc/lock:
hb is currently running GRASS in selected mapset (file
$LOCATION/user1/.gislock found). Concurrent use not allowed.
}}}

user1 is drwxr-xr-x, so the Permission denied comes from the fact that
etc/lock can't write the .gislock file, not that my current user account
already has a copy there. (I checked, the file doesn't exist)

So that Permission denied needs to be better caught I guess.
or improve G!__mapset_permissions() or G!__mapset_permissions2() and it's
a case of the error message being a bit too overenthusiastic?

if I start GRASS with -tcltk the mapset is simply missing from the list
(greyed out would be nicer if $MAPSET/WIND exists but we don't own it). I
don't have wx on this machine to test the wxGUI startup, nor did I test
changing mapsets with g.mapset. And no idea what happens if you force the
mapset change with g.gisenv.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755&gt;
GRASS GIS <http://grass.osgeo.org>

#755: trying to start in mapset you don't own gives wrong error msg
----------------------+-----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Resolution: | Keywords: mapset access
  Platform: Linux | Cpu: x86-32
----------------------+-----------------------------------------------------
Old description:

Hi,

if I do "chmod 999.999 $LOCATION/user1" to make my user account not the
owner of a mapset, and then start grass with the full path to the mapset
I get this error:

{{{
Cleaning up temporary files ...
Starting GRASS ...
$LOCATION/user1/.gislock: Permission denied
ERROR: $GISBASE/etc/lock:
hb is currently running GRASS in selected mapset (file
$LOCATION/user1/.gislock found). Concurrent use not allowed.
}}}

user1 is drwxr-xr-x, so the Permission denied comes from the fact that
etc/lock can't write the .gislock file, not that my current user account
already has a copy there. (I checked, the file doesn't exist)

So that Permission denied needs to be better caught I guess.
or improve G!__mapset_permissions() or G!__mapset_permissions2() and
it's a case of the error message being a bit too overenthusiastic?

if I start GRASS with -tcltk the mapset is simply missing from the list
(greyed out would be nicer if $MAPSET/WIND exists but we don't own it). I
don't have wx on this machine to test the wxGUI startup, nor did I test
changing mapsets with g.mapset. And no idea what happens if you force the
mapset change with g.gisenv.

Hamish

New description:

Hi,

if I do "chmod 999.999 $LOCATION/user1" to make my user account not the
owner of a mapset, and then start grass with the full path to the mapset I
get this error:

{{{
Cleaning up temporary files ...
Starting GRASS ...
$LOCATION/user1/.gislock: Permission denied
ERROR: $GISBASE/etc/lock:
hamish is currently running GRASS in selected mapset (file
$LOCATION/user1/.gislock found). Concurrent use not allowed.
}}}

user1 is drwxr-xr-x, so the Permission denied comes from the fact that
etc/lock can't write the .gislock file, not that my current user account
already has a copy there. (I checked, the file doesn't exist)

So that Permission denied needs to be better caught I guess.
or improve G!__mapset_permissions() or G!__mapset_permissions2() and it's
a case of the error message being a bit too overenthusiastic?

if I start GRASS with -tcltk the mapset is simply missing from the list
(greyed out would be nicer if $MAPSET/WIND exists but we don't own it). I
don't have wx on this machine to test the wxGUI startup, nor did I test
changing mapsets with g.mapset. And no idea what happens if you force the
mapset change with g.gisenv.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#755: trying to start in mapset you don't own gives wrong error msg
----------------------+-----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Resolution: | Keywords: mapset access
  Platform: Linux | Cpu: x86-32
----------------------+-----------------------------------------------------
Comment (by glynn):

Replying to [ticket:755 hamish]:

> if I do "chmod 999.999 $LOCATION/user1" to make my user account not the
owner of a mapset,

Do you mean "chown"?

> and then start grass with the full path to the mapset I get this error:
>
{{{
$LOCATION/user1/.gislock: Permission denied
ERROR: $GISBASE/etc/lock:
hamish is currently running GRASS in selected mapset (file
$LOCATION/user1/.gislock found). Concurrent use not allowed.
}}}

Making wild guesses as to the cause of a failure then presenting such
guesses as facts is a common problem in GRASS. Unfortunately, I my
experience suggests that this isn't going to change any time soon.

> user1 is drwxr-xr-x, so the Permission denied comes from the fact that
etc/lock can't write the .gislock file, not that my current user account
already has a copy there. (I checked, the file doesn't exist)

If etc/lock fails with an exit code of 1 (which is what you get if it
calls G_fatal_error()), etc/Init.sh prints the above message. The possible
reasons for an exit code of 1 are:

  1. Wrong number of arguments or the second argument isn't a valid
integer.
  2. Lock file exists, contains a PID, and kill(pid, 0) either succeeded or
failed for a reason other than ESRCH.
  3. Lock file creation failed.
  4. Writing the PID to the lock file failed.

For 1, 3, and 4, it also generates an error message.

It cannot fail with an exit code other than 0 or 1.

> So that Permission denied needs to be better caught I guess.

Either etc/Init.sh needs to be changed to print e.g. "failed to create
lockfile" rather than making unsubstantiated claims as to *why* lockfile
creation failed, or etc/lock needs to be changed to provide information as
to why it failed.

> or improve G!__mapset_permissions() or G!__mapset_permissions2() and
it's a case of the error message being a bit too overenthusiastic?

Those functions aren't used by etc/lock.

Also, those functions don't print error messages, they just return a
status code: 1 for success, 0 for found but not owned by the current user,
-1 for couldn't stat() or isn't a directory.

Any error message is printed by G!__gisinit(): "permission denied" for 0,
"not found" for -1. Those messages are reasonably accurate, although
someone who is used to standard error messages (perror()) may be mislead
by the similarity with the standard messages for EPERM and ENOENT
respectively.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#755: trying to start in mapset you don't own gives wrong error msg
----------------------+-----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Resolution: | Keywords: mapset access
  Platform: Linux | Cpu: x86-32
----------------------+-----------------------------------------------------
Comment (by hamish):

> Replying to [ticket:755 hamish]:
>
> > if I do "chmod 999.999 $LOCATION/user1" to make my user
> > account not the owner of a mapset,

Replying to [comment:2 glynn]:
> Do you mean "chown"?

yeah

> > and then start grass with the full path to the mapset I get
> > this error:
> >
{{{
> $LOCATION/user1/.gislock: Permission denied
> ERROR: $GISBASE/etc/lock:
> hamish is currently running GRASS in selected mapset (file
$LOCATION/user1/.gislock found). Concurrent use not allowed.
}}}

> Making wild guesses as to the cause of a failure then presenting
> such guesses as facts is a common problem in GRASS.

In this case it seems that something is wrong in lock.c, the above message
should only be presented when the lock file does exist and the value in it
matches a current PID. So not so wild a guess after all.

The alternate failure error which alone should have been triggered gives
some handy suggestions about why you might get a filesystem error but
doesn't state as fact why it happened.

quoth:
{{{
     locked = 0;
     if ((lock = open(file, 0)) >= 0) { /* file exists */
         G_sleep(1); /* allow time for file creator to write
its pid */
         if (read(lock, &pid, sizeof pid) == sizeof pid)
             locked = find_process(pid);
         close(lock);
     }
     if (locked)
         exit(1);

     if ((lock = creat(file, 0666)) < 0) {
         perror(file);
         G_fatal_error("%s: ", argv[0]);
     }
     if (write(lock, &lockpid, sizeof lockpid) != sizeof lockpid)
         G_fatal_error("%s: can't write lockfile %s (disk full?
Permissions?)",
                       argv[0], file);
     close(lock);
     exit(0);
}}}

> If etc/lock fails with an exit code of 1 (which is what you get
> if it calls G_fatal_error()), etc/Init.sh prints the above
> message.
...
> It cannot fail with an exit code other than 0 or 1.

do you mean that as strict policy, or interpreting what was seen in the
code?

> or etc/lock needs to be changed to provide information as to
> why it failed.

For now in trunk I've modified lock.c to exit with code 2 if the lockfile
validly exists, absolving the init.sh error message and freeing exit code
1 to indicate a generic error.

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#755: trying to start in mapset you don't own gives wrong error msg
----------------------+-----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Resolution: | Keywords: mapset access
  Platform: Linux | Cpu: x86-32
----------------------+-----------------------------------------------------
Comment (by glynn):

Replying to [comment:3 hamish]:

> In this case it seems that something is wrong in lock.c, the above
message should only be presented when the lock file does exist and the
value in it matches a current PID. So not so wild a guess after all.

The only thing that's wrong in lock.c is that it doesn't offer any
opportunity for the caller to distinguish between the various ways it can
fail. Its exit code is 0 for success, 1 for any kind of failure.

> > If etc/lock fails with an exit code of 1 (which is what you get
> > if it calls G_fatal_error()), etc/Init.sh prints the above
> > message.
> ...
> > It cannot fail with an exit code other than 0 or 1.
>
> do you mean that as strict policy, or interpreting what was seen in the
code?

I mean that there's no way that it can return any other exit code.

> > or etc/lock needs to be changed to provide information as to
> > why it failed.
>
> For now in trunk I've modified lock.c to exit with code 2 if the
lockfile validly exists, absolving the init.sh error message and freeing
exit code 1 to indicate a generic error.

I've updated grass.py accordingly in r39252.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#755: trying to start in mapset you don't own gives wrong error msg
----------------------+-----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: closed
  Priority: minor | Milestone: 6.4.0
Component: default | Version: svn-develbranch6
Resolution: fixed | Keywords: mapset access
  Platform: Linux | Cpu: x86-32
----------------------+-----------------------------------------------------
Changes (by hamish):

  * status: new => closed
  * resolution: => fixed

Comment:

> I've updated grass.py accordingly in r39252.

thanks

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/755#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>