Skip to content
This repository was archived by the owner on Nov 17, 2023. It is now read-only.

Fix that lnd might not be detected on Mac OS due to misaligned ps output#124

Closed
Empact wants to merge 1 commit into
LN-Zap:masterfrom
Empact:fix/lnd-detection
Closed

Fix that lnd might not be detected on Mac OS due to misaligned ps output#124
Empact wants to merge 1 commit into
LN-Zap:masterfrom
Empact:fix/lnd-detection

Conversation

@Empact
Copy link
Copy Markdown
Contributor

@Empact Empact commented Dec 24, 2017

On mac, the ps output can be misaligned which prevents lnd from being detected, e.g. when some VSZ field is longer than expected.

The default ps-node lookup is via ps lx, which includes:

  -l Display information associated with the following keywords:
    uid, pid, ppid, flags, cpu, pri, nice, vsz=SZ, rss, wchan,
    state=S, paddr=ADDR, tty, time, and command=CMD.

Here's some example misaligned output from ps lx

  501 19344     1   0   4  0  4376668   1804 -      S      ??    0:01.23 /System/Library/PrivateFrameworks/IMDPersistence.framework/IMAutomaticHistoryDeletionAgent.a
  501 33800     1   0   4  0 21481005020 172224 -      S      ??    1:35.79 /Applications/GitX.app/Contents/MacOS/GitX
  501 34520     1   0   4  0  4453292   3568 -      S      ??    0:04.03 /System/Library/CoreServices/navd
  501 34689   352   0  31  0  5668268  43616 -      S      ??    2:01.09 /Applications/Slack.app/Contents/Frameworks/Slack Helper.app/Contents/MacOS/Slack Helper --t

Many of these fields are unnecessary because we're looking up by name only,
so we can lookup using only the -o pid,command output.

neekey/table-parser#11
neekey/ps#64

@Empact
Copy link
Copy Markdown
Contributor Author

Empact commented Dec 25, 2017

Updated to include specific output columns. This is tested to work on my machine.

Opened something against ps-node as well. neekey/ps#69

@Empact Empact changed the title Lookup lnd using 'ps x' rather than 'ps lx' Lookup lnd using ps x -o pid,command rather than ps lx Dec 25, 2017
@Empact Empact force-pushed the fix/lnd-detection branch 3 times, most recently from f75238c to baafcb8 Compare December 29, 2017 07:04
@Empact Empact changed the title Lookup lnd using ps x -o pid,command rather than ps lx Fix that lnd might not be detected on Mac OS due to misaligned ps output Dec 29, 2017
@JimmyMow
Copy link
Copy Markdown
Member

@Empact we should test this on other machines just in case, or wait to see how neekey/ps#69 gets handled

@Empact Empact force-pushed the fix/lnd-detection branch 2 times, most recently from d7d4a11 to 833b71d Compare January 5, 2018 21:19
@Empact Empact force-pushed the fix/lnd-detection branch from 833b71d to cd6c1b8 Compare January 10, 2018 08:40
@Empact Empact force-pushed the fix/lnd-detection branch from cd6c1b8 to d21401e Compare January 22, 2018 09:10
@Empact Empact force-pushed the fix/lnd-detection branch from d21401e to ad3f257 Compare March 17, 2018 02:53
@Empact Empact force-pushed the fix/lnd-detection branch from ad3f257 to 2184f87 Compare June 6, 2018 21:42
@Empact Empact force-pushed the fix/lnd-detection branch from 2184f87 to 0f6621e Compare June 15, 2018 23:39
`ps x -o pid,command` rather than `ps lx`

The default ps-node lookup is via ps lx, which includes:

  -l Display information associated with the following keywords:
    uid, pid, ppid, flags, cpu, pri, nice, vsz=SZ, rss, wchan,
    state=S, paddr=ADDR, tty, time, and command=CMD.

On mac, the `ps l` output can be misaligned, e.g. when some VSZ field is
longer than expected.

Many of these fields are unnecessary because we're looking up by name only,
so we can lookup using only the command fields of ps.
https://gist.github.com/ivankovacevic/9918272

More on the issue with ps-node:
neekey/table-parser#11
neekey/ps#64
@Empact Empact force-pushed the fix/lnd-detection branch from 0f6621e to 90236da Compare August 17, 2018 06:19
@Empact
Copy link
Copy Markdown
Contributor Author

Empact commented Aug 17, 2018

Rebased, this is still an issue fyi. Will ping neekey again.

@coveralls
Copy link
Copy Markdown

coveralls commented Aug 17, 2018

Pull Request Test Coverage Report for Build 3735

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 12.311%

Changes Missing Coverage Covered Lines Changed/Added Lines %
app/lib/lnd/util.js 0 1 0.0%
Totals Coverage Status
Change from base Build 3731: 0.0%
Covered Lines: 466
Relevant Lines: 3334

💛 - Coveralls

@mrfelton
Copy link
Copy Markdown
Member

We no longer attempt to detect wether or not lnd is already running on the user's machine as there is no longer a hard requirement that our internal instance of lnd must be the only one running.

@mrfelton mrfelton closed this Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants