Skip to content

Fix runtime attach for current user#1819

Merged
SylvainJuge merged 6 commits intoelastic:masterfrom
SylvainJuge:wip-fix-for-runtime-attach
May 20, 2021
Merged

Fix runtime attach for current user#1819
SylvainJuge merged 6 commits intoelastic:masterfrom
SylvainJuge:wip-fix-for-runtime-attach

Conversation

@SylvainJuge
Copy link
Member

@SylvainJuge SylvainJuge commented May 18, 2021

What does this PR do?

When using runtime attach as root user, it seems that in some circumstances the attach process hangs likely due to a sudo interactive input. In this case, the target JVM also runs as root, thus trying to use sudo is not even required at all.

When this happens with agent 1.23.0, the following stack trace is visible in a thread dump:

   java.lang.Thread.State: WAITING (on object monitor)
        at java.lang.Object.wait(java.base@11.0.9.1/Native Method)
        - waiting on <0x00000000c65e0ab0> (a java.lang.ProcessImpl)
        at java.lang.Object.wait(java.base@11.0.9.1/Object.java:328)
        at java.lang.ProcessImpl.waitFor(java.base@11.0.9.1/ProcessImpl.java:495)
        - waiting to re-lock in wait() <0x00000000c65e0ab0> (a java.lang.ProcessImpl)
        at co.elastic.apm.attach.UserRegistry$User.canSwitchToUser(UserRegistry.java:153)
        at co.elastic.apm.attach.UserRegistry$User.of(UserRegistry.java:144)
        at co.elastic.apm.attach.UserRegistry$User.access$100(UserRegistry.java:130)
        at co.elastic.apm.attach.UserRegistry.get(UserRegistry.java:125)
        at co.elastic.apm.attach.UserRegistry.getCurrentUser(UserRegistry.java:90)
        at co.elastic.apm.attach.AgentAttacher.<init>(AgentAttacher.java:73)
        at co.elastic.apm.attach.AgentAttacher.main(AgentAttacher.java:131)

What this PR does (at least as time of writing) is just making sure that we do not use sudo whenever the current user is already the one from the target JVM. Fixing the blocking behavior will be a second step.

Checklist

  • ensure that we redirect IO in case the underlying command use stdin/stderr (would have allowed to debug this easily).
  • adding a timeout when waiting for sudo execution to prevent such blocking behavior + properly log the issue
    --> not doable as waitFor with timeout variant has been added in Java 8.
  • This is a bugfix
@ghost
Copy link

ghost commented May 18, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #1819 updated

  • Start Time: 2021-05-20T14:53:22.294+0000

  • Duration: 58 min 32 sec

  • Commit: 91e3d5d

Test stats 🧪

Test Results
Failed 0
Passed 2133
Skipped 18
Total 2151

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 2133
Skipped 18
Total 2151

@SylvainJuge SylvainJuge marked this pull request as ready for review May 20, 2021 12:33
@SylvainJuge SylvainJuge merged commit 1dabfb9 into elastic:master May 20, 2021
@SylvainJuge SylvainJuge deleted the wip-fix-for-runtime-attach branch May 20, 2021 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants