Opened 10 years ago

Closed 10 years ago

#755 closed defect (fixed)

trying to start in mapset you don't own gives wrong error msg

Reported by: hamish Owned by: grass-dev@…
Priority: minor Milestone: 6.4.0
Component: Default Version: svn-develbranch6
Keywords: mapset access Cc:
CPU: x86-32 Platform: Linux

Description (last modified by hamish)

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

Change History (5)

comment:1 Changed 10 years ago by hamish

Description: modified (diff)

comment:2 in reply to:  description ; Changed 10 years ago by glynn

Replying to 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.

comment:3 in reply to:  2 ; Changed 10 years ago by hamish

Replying to hamish:

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

Replying to 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

comment:4 in reply to:  3 Changed 10 years ago by glynn

Replying to 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.

comment:5 Changed 10 years ago by hamish

Resolution: fixed
Status: newclosed

I've updated grass.py accordingly in r39252.

thanks

Hamish

Note: See TracTickets for help on using tickets.