Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

seems like nodejs console race condition bug when sudo #56837

Open
loynoir opened this issue Jan 31, 2025 · 6 comments
Open

seems like nodejs console race condition bug when sudo #56837

loynoir opened this issue Jan 31, 2025 · 6 comments

Comments

@loynoir
Copy link

loynoir commented Jan 31, 2025

Version

v23.7.0

Platform


Subsystem

No response

What steps will reproduce the bug?

bug

seems like nodejs console race condition bug when sudo

version

node v23.7.0

yarn 4.6.0

reproduce

https://github.com/loynoir/reproduce-yarn-berry-6667

  "scripts": {
    "reproduce": "yarn workspaces foreach --all --exclude . run reproduce",
  "scripts": {
    "reproduce": "sudo node ./simulate.mjs",
import { readFile } from "node:fs/promises";
import { stderr, stdout } from "node:process";

/** @type {Array<{ type: "stdout" | "stderr"; value: string }>} */
const data = JSON.parse(
    await readFile("./simulate.json", "utf8"),
);

for (const el of data) {
    switch (el.type) {
        case "stdout":
            stdout.write(el.value);
            break;
        case "stderr":
            stderr.write(el.value);
            break;
        default:
            throw new Error();
    }
}
[root@e556503f99d5 app]# yarn reproduce
[hello]: Process started
[hello]: #0 building with "default" instance using docker driver
                                                                [hello]:
                                                                         [hello]: #1 [internal] load build definition from Dockerfile
                                                                                                                                     [hello]: #1 transferring dockerfile:
                                                                                                                                                                         [hello]: #1 transferring dockerfile: 163B done
                                      [hello]: #1 DONE 0.4s
                                                           [hello]:
                                                                    [hello]: #2 [internal] load metadata for public.ecr.aws/docker/library/hello-world:latest@sha256:d715f14f9eca81473d9112df50457893aa4d099adeb4729f679006bf5ea12407
                                                    [hello]: #2 DONE 0.7s
                                                                         [hello]:
                                                                                  [hello]: #3 [internal] load .dockerignore
                                                                                                                           [hello]: #3 transferring context:
                                                                                                                                                            [hello]: #3 transferring context: 2B done
                    [hello]: #3 DONE 0.4s
                                         [hello]:
                                                  [hello]: #4 [1/1] FROM public.ecr.aws/docker/library/hello-world:latest@sha256:d715f14f9eca81473d9112df50457893aa4d099adeb4729f679006bf5ea12407
                [hello]: #4 CACHED
                                  [hello]:
                                           [hello]: #5 exporting to image
                                                                         [hello]: #5 exporting layers done
                                                                                                          [hello]: #5 writing image sha256:74cc54e27dc41bb10dc4b2226072d469509f2f22f1a3ce74f4a59661a1d44602 0.0s done
                                    [hello]: #5 DONE 0.2s
                                                         [hello]: Process exited (exit code 0), completed in 0s 160ms
Done in 0s 165ms
[root@e556503f99d5 app]#

related

yarnpkg/berry#6667

How often does it reproduce? Is there a required condition?

sudo

What is the expected behavior? Why is that the expected behavior?

console OK

What do you see instead?

console mess up

Additional information

No response

@loynoir
Copy link
Author

loynoir commented Jan 31, 2025

Not sure it is a yarn berry bug, or nodejs bug + deno bug.

Verify Deno have same console mess up behavior without below workaround.

async function cp_output_workaround(cp: Deno.ChildProcess) {
  await Promise.all([
    (async () => {
      for await (const el of cp.stdout) {
        Deno.stdout.write(el);
      }
    })(),
    (async () => {
      for await (const el of cp.stderr) {
        Deno.stderr.write(el);
      }
    })(),
  ]);
}

I found it working.

But not really understand why sudo console mess up, without workaround.

@loynoir
Copy link
Author

loynoir commented Feb 1, 2025

summary

Until now, have not yet been confirmed as bug by nodejs and yarn.

Below is summary based on my current understanding of this problem.

I think

  • This is a nodejs + deno race condition bug when sudo

  • This may called yarn berry bug, depends on individual preference.

brief

https://github.com/loynoir/reproduce-yarn-berry-6667

  "scripts": {
    "reproduce": "yarn workspaces foreach --all --exclude . run reproduce",
    "workaround": "yarn workspaces foreach --all --exclude . run workaround",
    "simulate": "yarn workspaces foreach --all --exclude . run simulate"
  }
  "scripts": {
    "reproduce": "sudo node ./simulate.mjs",
    "workaround": "deno run -A ./workaround.ts",
    "simulate": "node ./simulate.mjs"
  }
script console
yarn reproduce mess up
yarn workaround OK
yarn simulate Ok
DISABLE_WORKAROUND=1 yarn workaround mess up

node

Because

  • yarn simulate console OK

Which means

  • yarn berry logic is fine.

But

  • yarn reproduce console mess up.

  • yarn workaround console OK

Which means

  • There is a way, I have not yet have time to extract logic out from yarn berry, leads to sudo XXX console is not isomprphic to XXX console.

  • There is also a way, let sudo XXX console isomprphic to XXX console

  • sudo XXX console is not always isomprphic to XXX console

I think

  • This is a nodejs race condition bug when sudo.

yarn

Because

  • yarn reproduce console mess up.

  • yarn workaround console OK

  • yarn simulate console OK

I think

  • This may called yarn berry bug, depends on individual preference.

deno

Because

  • yarn workaround console OK

  • DISABLE_WORKAROUND=1 yarn workaround console mess up

I think

  • This is a deno race condition bug when sudo without async iterator.

@bnoordhuis
Copy link
Member

Node switches the pty to non-blocking mode, often to raw (uncooked) mode as well, and sudo doesn't expect that. Not really a bug in either program, just an unfortunate interaction.

I don't think this is really actionable. Just don't run sudo from your package.json, that's a terrible idea in the first place.

@loynoir
Copy link
Author

loynoir commented Feb 3, 2025

@bnoordhuis

The sudo console problem was found, when calling docker build task within new devcontainer.

Currently, I'm really interested on minimal reproduce of prefixer leads to sudo console mess up.

Below is a prefixer prototype, I cannot reproduce a mess up console.

import { spawn } from "node:child_process";
import { stderr, stdout } from "node:process";

/**
 * @param {string} prefix
 * @param {NodeJS.WritableStream} w
 * @returns {(chunk:Buffer)=>string}
 */
function prefixer(prefix, w) {
  /**
   * @param {Buffer} chunk
   */
  const handle_lead = (chunk) => {
    w.write(prefix + chunk.toString().replaceAll("\n", `\n${prefix}`));

    handle = handle_mid;
  };
  /**
   * @param {Buffer} chunk
   */
  const handle_mid = (chunk) => {
    w.write(chunk.toString().replaceAll("\n", `\n${prefix}`));
  };

  let handle = handle_lead;
  return (chunk) => handle(chunk);
}

const workspaces = [
  {
    name: "hello",
    cmd: 'sudo',
    args: ["node", "./simulate.mjs"],
    cwd: "./pkgs/hello",
  },
];

await Promise.all(
  workspaces.map((el) => {
    const prefix = `[${el.name}]: `;
    const cp = spawn(el.cmd, el.args, { cwd: el.cwd, stdio: "pipe" });

    cp.stdout.addListener("data", prefixer(prefix, stdout));
    cp.stderr.addListener("data", prefixer(prefix, stderr));
  }),
);
$ node ./prefixer.mjs
[hello]: #0 building with "default" instance using docker driver
[hello]:
[hello]: #1 [internal] load build definition from Dockerfile
[hello]: #1 transferring dockerfile:
[hello]: #1 transferring dockerfile: 163B done
[hello]: #1 DONE 0.4s
[hello]:
[hello]: #2 [internal] load metadata for public.ecr.aws/docker/library/hello-world:latest@sha256:d715f14f9eca81473d9112df50457893aa4d099adeb4729f679006bf5ea12407
[hello]: #2 DONE 0.7s
[hello]:
[hello]: #3 [internal] load .dockerignore
[hello]: #3 transferring context:
[hello]: #3 transferring context: 2B done
[hello]: #3 DONE 0.4s
[hello]:
[hello]: #4 [1/1] FROM public.ecr.aws/docker/library/hello-world:latest@sha256:d715f14f9eca81473d9112df50457893aa4d099adeb4729f679006bf5ea12407
[hello]: #4 CACHED
[hello]:
[hello]: #5 exporting to image
[hello]: #5 exporting layers done
[hello]: #5 writing image sha256:74cc54e27dc41bb10dc4b2226072d469509f2f22f1a3ce74f4a59661a1d44602 0.0s done
[hello]: #5 DONE 0.2s
[hello]: %

@bnoordhuis
Copy link
Member

Your second example doesn't use process.stdin or process.stdout so node doesn't switch to raw mode.

In case my first comment wasn't clear: it's neither a node bug or a sudo bug. Just accept that they don't mingle well and move on.

@loynoir
Copy link
Author

loynoir commented Feb 3, 2025

Still cannot reproduce a mess up console.

process.stdin.setRawMode(true)
await Promise.all(
  workspaces.map((el) => {
    const prefix = `[${el.name}]: `;
    const cp = spawn(el.cmd, el.args, { cwd: el.cwd, stdio: "pipe" });

    cp.stdout.addListener("data", prefixer(prefix, process.stdout));
    cp.stderr.addListener("data", prefixer(prefix, process.stderr));
  }),
);

Could you show me the code example on how to reproduce a mess up console, please?

Thank you very much. @bnoordhuis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants