Hi Fat-Zer,
although it is clear that Russell's patch will not be able to solve all of
the problems mentioned in 1766, it is a good step forward - quickly and
simply solving one particular problem. Therefore, there is good reason to
merge it.
Cheers
--
Slávek
On Friday 08 of February 2019 00:10:24 Fat-Zer wrote:
> Hi,
>
> Note that this is essentially a lighter version of the issue in bug
> 1766.
>
> I'm digging it right now, but a simple adding of the DISPLAY won't fix
> it for me because as for now there are actually 2 instances for each
> of two screens (4 in total, see the bug for details) are fighting over
> control. Having a squared-number-of-screen kdesktop_lock running all
> the time just looks messed up. Also they conflict with each other when
> locking a desktop from a «wrong screen».
>
> So, I'd rather get rid of all of those constantly running
> kdesktop_lock(s) and leave only one or one-for-each-screen at most.
> But it requires some thine and gentle tweaking to achieve it and not
> compromise the security/stability.
>
> I've tried to rearrange things in the `main()` at first (to lock and
> wait for a signal from kdesktop before forking), but it didn't gone
> specifically well, actually I've got caught up into some sort of
> cabbage-goat-wolf puzzle but with a flesh-eating cabbage this time...
>
> As for now I'm looking forward for some changes in
> kdesktop/kdesktop_lock or inter-kdesktop communications. Also I was
> going to commit some refactoring on the kdesktop/lock/main.cc because
> as for now that mess is unmodifiable (on rather high level)...
>
> чт, 7 февр. 2019 г. в 22:41, Slávek Banko <slavek.banko@xxxxxxx>:
> > On Thursday 07 of February 2019 16:52:21 Russell Brown wrote:
> > > Hi,
> > >
> > > I don't know how closely the devs watch the Bug list but I believe
> > > I've fixed Bug 2998.
> > >
> > > As this bug results in kdesktop/kdesktop_lock eating CPU and going
> > > unresponsive; might I humbly request that the fix is incorporated
> > > in the next possible/dev version.
> > >
> > > As the code I posted on bugzilla used a crude fixed buffer, here's
> > > a cleaned up version that uses malloc.
> > >
> > > *** /tmp/tdebase-trinity-14.0.6~pre38/kdesktop/lock/main.cc Thu
> > > Feb 7 15:48:21 2019 ---
> > > /usr/tmp/tdebase-trinity-14.0.6~pre38/kdesktop/lock/main.cc Sun May
> > > 20 19:41:55 2018 ***************
> > > *** 325,336 ****
> > > #endif
> > > }
> > >
> > > ! char *locknameroot="kdesktop_lock_lockfile.";
> > > ! char *lockfilename = (char*)malloc(strlen(locknameroot) +
> > > strlen(getenv("DISPLAY")) + 1); !
> > > strcpy(lockfilename,locknameroot);
> > > ! strcat(lockfilename,getenv("DISPLAY"));
> > > !
> > > ! TDELockFile lock(locateLocal("tmp", lockfilename));
> > > lock.setStaleTime(0);
> > > TDELockFile::LockResult lockRet = lock.lock();
> > > if (lockRet != TDELockFile::LockOK) {
> > > --- 325,331 ----
> > > #endif
> > > }
> > >
> > > ! TDELockFile lock(locateLocal("tmp",
> > > "kdesktop_lock_lockfile")); lock.setStaleTime(0);
> > > TDELockFile::LockResult lockRet = lock.lock();
> > > if (lockRet != TDELockFile::LockOK) {
> >
> > Hi Russell,
> >
> > your observation gives a very clear sense. The name of the lock does
> > not distinguish between $DISPLAY and therefore two separate
> > kdesktop_lock processes struggles for one file. Good conclusion!
> >
> > Because locateLocal expects a TQString value as argument, I suggest
> > using this fact and make the patch simpler:
> >
> > --- a/kdesktop/lock/main.cc
> > +++ b/kdesktop/lock/main.cc
> > @@ -325,7 +325,7 @@
> > #endif
> > }
> >
> > - TDELockFile lock(locateLocal("tmp",
> > "kdesktop_lock_lockfile")); + TDELockFile
> > lock(locateLocal("tmp",
> > TQString("kdesktop_lock_lockfile.%1").arg(getenv("DISPLAY"))));
> > lock.setStaleTime(0);
> > TDELockFile::LockResult lockRet = lock.lock();
> > if (lockRet != TDELockFile::LockOK) {
> >
> >
> > If you want to get involved, the best way is to register on TDE Gitea
> > Workspace and create a pull-request. See:
> >
> > https://wiki.trinitydesktop.org/TDE_Gitea_Workspace
> >
> > Note: Pull-requests should always be on the master branch. Core
> > developers then take care of a backport patch into a stable branch.
> >
> > Note1: By the way, in your proposed code is missing free, so this
> > would leads to memory leaks.
> >
> > Cheers
> > --
> > Slávek
>
---------------------------------------------------------------------
To unsubscribe, e-mail: trinity-devel-unsubscribe@xxxxxxxxxxxxxxxxxxxxxxxxxx
For additional commands, e-mail: trinity-devel-help@xxxxxxxxxxxxxxxxxxxxxxxxxx
Read list messages on the web archive: http://trinity-devel.pearsoncomputing.net/
Please remember not to top-post: http://trinity.pearsoncomputing.net/mailing_lists/#top-posting
