Menu

#81 Parcellite cannot run for multiple users

Stable_Release
closed-fixed
None
5
2013-08-17
2013-02-27
Tomas Psika
No

Since version 1.0.2_rc6 (on gentoo) parcellite is not able to run (as daemon) on different sessions of different users. It cannot open fifos (they are not created on startup) and sometimes segfaults.

/proc filesystem discovery does to not work within last versions of parcellite. It does not distinguish different users on system. Maybe appindicator functionality could be affected too.

This patch solves the application startup issue within development version (svn r349):
http://psika.cz/files/parcellite-svn-r349-fix-multiple-instances.patch

Config file ~/.config/parcellite/parcelliterc attached.

1 Attachments

Discussion

  • rickyrockrat

    rickyrockrat - 2013-03-11

    Tomas,
    There are other issues with Daemon mode also that I need to fix. I like the idea of PID file creation, however it looks like this will can fail on the second invocation of Parcellite, since I don't see the deletion of the PID file.

    I see the issue with multiple users, though, and I think I see another solution, and that is to qualify the session with the user name. Thanks for the patch.

     
  • Tomas Psika

    Tomas Psika - 2013-03-11

    Deletion of the PID file seems not be important and necessary. Exclusive lock (LOCK_EX) guarantees only atomicity in locking (only one process can obtain file lock). And kernel should close all file descriptors on process termination (irrespective of signal type).

    Qualifying sessions with user name via "grepping" effective user id in /proc/*/status was another meaningful solution, but i was too lazy to implement it ;-)

    Good luck with your work. And thanks for your software, I like it very much.

     
  • rickyrockrat

    rickyrockrat - 2013-03-11

    Please check out svn version 374, and see if it fixes the issue. I've seen PID files left over when something catastrophic happens (i.e. sudden power loss), in which case those pid files will be laying about, so there needs to be a cleanup function somewhere.

    I prefer to not have to deal with these unusual, highly unlikely scenarios even possible, because they cause one to go bald while pulling one's hair out.

    I don't think the kernel removes the files from the filesystem, so if parcellite doesn't do it, they will accumulate, which I find very messy.

    If we can't get it to work with the dynamic user as implemented in 374, I'll revisit your patch.

    Make sure you use this repository:
    http://svn.code.sf.net/p/parcellite/code/trunk

    It was wrong on the documentation page.

     

    Last edit: rickyrockrat 2013-03-11
  • Tomas Psika

    Tomas Psika - 2013-03-12

    No cleanup function is necessary (but suitable of course). It does not matter if PID file being left after process termination. You only need to lock the file by one running process and not by the other one. That does the patch.

    Your changes does not work (on my box), two issues found:

    1. /proc/PID/environ could not be opened for reading by different user because of 0400 file permissions. You could chmod this file by first instance, but it could be considered as security vulnerability (exposing process environment on system).
    2. segfaulting if /proc/PID/environ file could not be read. Char pointer *env should be NULL-initialized by declaration in get_value_from_env().

    Hope this helps.

     
  • rickyrockrat

    rickyrockrat - 2013-03-12

    I get the point on flock. For some reason I was thinking how init scripts handle PIDs.

    I don't really have multiple logins on my dev box. Thanks for testing this for me. Try svn 377 (latest) It should address the permission issue and the segfault.

     

    Last edit: rickyrockrat 2013-03-12
    • Tomas Psika

      Tomas Psika - 2013-03-12

      It does not work. Sample console output follows:

      --
      Unable to open '/proc/30940/environ' for PID 30940
      parcellite found
      ..
      Error opening fifo. Exit
      --

      Several issues found:

      1) missing environment variable

      Environment variable USERNAME does not exist. Several linux-based systems (and shells) do not initialize this variable at all. There is no standard for environment variables, maybe POSIX "knows" LOGNAME. And USER is often used for storing effective user id. USERNAME is only sometimes used for storing real user id. It seems not safe to use these environment variables.

      2) get_value_from_env()

      In case of get_value_from_env() returns nothing (NULL) (permission issue or no environment variable set), is_current_user() returns 1 nevertheless. Missing assigment "rtn=0" before "goto done".

      3) get_username()

      User home directory basename could have nothing to do with username. But on "user-friendly" distributions cases it could work ;).

       
  • rickyrockrat

    rickyrockrat - 2013-03-12
    • status: open --> pending-fixed
     
  • lkraav

    lkraav - 2013-04-03

    There is another consideration regarding multiple running copies that cannot be addressed by only checking username. I run two Xorg concurrent sessions with my user, one extra for displaylink USB monitor. I'd like to have parcellite running on both X sessions. So there should also be another qualifier, maybe based on DISPLAY variable.

     
  • rickyrockrat

    rickyrockrat - 2013-07-13

    Hmm. Do you have a XDG_SESSION_COOKIE env variable in each of your X sessions, and is it different for each one? I assume it is, and if so, I can just use that. lkraav, Tomas, can you check?

     
  • rickyrockrat

    rickyrockrat - 2013-07-13

    Tomas,
    I just checked in SVN 400, and I need to know if it fixes your multi-user issues. If so, I think I have a simple solution for Ikraav's scenario, too.

    Thanks.

     
    • Tomas Psika

      Tomas Psika - 2013-08-15

      Just checked svn r400. Nevertheless does not work!

      Only because of incorrect usage of getpwuid(). See manual page:

      """
      The return value may point to a static area, and may be overwritten by subsequent calls to getpwent(3), getpwnam(), or getpwuid(). (Do not pass the returned pointer to free(3).)
      """

      By calling getpwuid() repeatedly, string IN THE STATIC AREA is modified and returned, so next call to g_strcmp0(username,user) ALWAYS returns 0, because *username and *user links to the same static memory!

      Just fix this in get_username_pid() or use reentrant version getpwuid_r():

      - return p->pw_name;
      + return g_strdup(p->pw_name);

      And do not forget to g_free() returned strings inside is_current_user() to avoid memory leak then.

      It works as you expected after this fix.

      I havent checked svn HEAD revision.

      It does not address lkraav issue of course.

       
  • rickyrockrat

    rickyrockrat - 2013-07-13

    lkraav, I just checked in 401, which should address the multiple X-Session issue. Please have a look and let me know. I'd like to release 1.1.5 with a working multi-user fix in place.

    -Thanks.

     
    • Tomas Psika

      Tomas Psika - 2013-08-15

      Briefly checked this revision (r401). It seems to use XDG_SESSION_COOKIE variable to check X login sessions. IMNSHO it depends on ConsoleKit which is now obsolete!

      If user has ConsoleKit on his box installed (I have), it could make sense, but...

       
  • rickyrockrat

    rickyrockrat - 2013-08-16

    Good catch on the getpwuid. I read the man page and saw the static note, but failed to accomodate it in the code. It uses XDG_SESSION_COOKIE, XDG_SEAT, and DISPLAY in that order. Hopefully that hits all the multiuser, same user, different X sessions. It's painful to test. Thanks for you patience. Try SVN 466.

     
    • Tomas Psika

      Tomas Psika - 2013-08-16

      SVN 466 works fine, at least on my box. I can run multiple instances on different X sessions of same user.

       
  • rickyrockrat

    rickyrockrat - 2013-08-17
    • status: pending-fixed --> closed-fixed
    • assigned_to: rickyrockrat
     
  • rickyrockrat

    rickyrockrat - 2013-08-17

    SVN 466 confirmed fix. Will show up in 1.1.7. I suspect lkraav is no longer commenting on this bug, since his username is gone from sourceforge.

     

Log in to post a comment.