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

test-snapshot-reproducible is failing in the main branch #53579

Closed
joyeecheung opened this issue Jun 25, 2024 · 8 comments
Closed

test-snapshot-reproducible is failing in the main branch #53579

joyeecheung opened this issue Jun 25, 2024 · 8 comments
Labels
repro-exists Issues with reproductions. snapshot Issues and PRs related to the startup snapshot test Issues and PRs related to the tests.

Comments

@joyeecheung
Copy link
Member

The snapshot reproducibility test has been failing in dynamically linked builds since yesterday. From CI history it was still green at cd8e61f (https://ci.nodejs.org/job/node-test-commit-linux-containered/44116/), but started failing no later than 4c730ae (https://ci.nodejs.org/job/node-test-commit-linux-containered/44117/), that means either one of the two commits in cd8e61f...4c730ae caused the regression, or some infra update happened yesterday caused the regression.

@joyeecheung
Copy link
Member Author

@joyeecheung
Copy link
Member Author

Looks like 4c730ae broke the test. Opened to revert it for now to make the CI green #53582.

@joyeecheung joyeecheung changed the title test-snapshot-reproducible is failing test-snapshot-reproducible is failing in the main branch Jun 25, 2024
nodejs-github-bot pushed a commit that referenced this issue Jun 25, 2024
Reason for revert: broke test-snapshot-reproducible.js in
dynamically linked builds in the CI.

This reverts commit 4c730ae.

PR-URL: #53582
Refs: #53579
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Stewart X Addison <sxa@redhat.com>
@joyeecheung
Copy link
Member Author

joyeecheung commented Jun 25, 2024

I haven't been able to reproduce this locally yet, but from the logs:

20:38:52     + [
20:38:52     +   {
20:38:52     +     offset: '0x40',
20:38:52     +     slice1: '000000b30b6c5ab8eea9aa31322e342e',
20:38:52     +     slice2: '000000fe0a668db8eea9aa31322e342e'
20:38:52     +   },
20:38:52     +   {
20:38:52     +     offset: '0x101640',
20:38:52     +     slice1: 'ec805d44660000000000000000a028c0',
20:38:52     +     slice2: 'ec805d44660000000000000000a03886'
20:38:52     +   },
20:38:52     +   {
20:38:52     +     offset: '0x101650',
20:38:52     +     slice1: 'd4655500000000000000000000000000',
20:38:52     +     slice2: '8c215600000000000000000000000000'
20:38:52     +   }
20:38:52     + ]

I suspect the unreproducibility comes from the hash of the flags - at least, it seems to be in the header of the main context snapshot.

@richardlau
Copy link
Member

I haven't been able to reproduce this in a "normal" build (Linux) but can with 4c730ae when building with configure --shared-openssl.

@VoltrexKeyva VoltrexKeyva added test Issues and PRs related to the tests. snapshot Issues and PRs related to the startup snapshot labels Jun 25, 2024
@RedYetiDev RedYetiDev added the repro-exists Issues with reproductions. label Jun 26, 2024
@joyeecheung
Copy link
Member Author

I am still unable to reproduce it with ./configure --shared-openssl or ./configure --with-intl=small-icu, neither on macOS nor on Ubuntu 23.04...but I could try logging into one of the containers to debug.

@joyeecheung
Copy link
Member Author

Ah, I could reproduce it now, it needs a separately installed shared openssl.

@legendecas
Copy link
Member

I can reproduce this on a local docker ubuntu container:

$ uname -a
Linux e3f5b61fd0a2 6.6.31-linuxkit #1 SMP Thu May 23 08:36:57 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux
$ ./configure --shared-openssl --shared-openssl-libpath=/usr/lib/aarch64-linux-gnu/ --shared-openssl-includes=/usr/include
$ make
$ ./out/Release/node test/parallel/test-snapshot-reproducible.js
0x0: Write magic 143da19
0x4: Write metadata
0x39: Write snapshot blob
0x185085: Write IsolateDataSerializeInfo
0x186538: Write EnvSerializeInfo
0x1873d1: Write CodeCacheInfo
0x0: Write magic 143da19
0x4: Write metadata
0x39: Write snapshot blob
0x185085: Write IsolateDataSerializeInfo
0x186538: Write EnvSerializeInfo
0x1873d1: Write CodeCacheInfo
node:assert:126
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ [
+   {
+     offset: '0x40',
+     slice1: '0001000000f63cf2004cb3bf2331322e',
+     slice2: '00010000004a3dfd284cb3bf2331322e'
+   },
+   {
+     offset: '0x111680',
+     slice1: '805d44660000000000000000707179fc',
+     slice2: '805d4466000000000000000000b9fbf6'
+   }
+ ]
- []
    at Object.<anonymous> (/workspace/test/parallel/test-snapshot-reproducible.js:69:8)
    at Module._compile (node:internal/modules/cjs/loader:1467:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1551:10)
    at Module.load (node:internal/modules/cjs/loader:1282:32)
    at Module._load (node:internal/modules/cjs/loader:1098:12)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:215:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:158:5)
    at node:internal/main/run_main_module:30:49 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: [
    {
      offset: '0x40',
      slice1: '0001000000f63cf2004cb3bf2331322e',
      slice2: '00010000004a3dfd284cb3bf2331322e'
    },
    {
      offset: '0x111680',
      slice1: '805d44660000000000000000707179fc',
      slice2: '805d4466000000000000000000b9fbf6'
    }
  ],
  expected: [],
  operator: 'deepStrictEqual'
}

Node.js v23.0.0-pre

@joyeecheung
Copy link
Member Author

joyeecheung commented Jun 27, 2024

Locally this patch fixes it for me (it is rather curious why this only shows up in dynamically linked builds, though..): https://chromium-review.googlesource.com/c/v8/v8/+/5662576
Trying it in the CI: https://ci.nodejs.org/job/node-test-commit-linux-containered/44167/

hubot pushed a commit to v8/v8 that referenced this issue Jul 2, 2024
Without initializing the max byte length field, any empty array
buffer captured in the snapshot can make the snapshot unreproducible.

Refs: nodejs/node#53579

Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/main@{#94754}
joyeecheung added a commit to joyeecheung/node that referenced this issue Jul 7, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: nodejs#53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
nodejs-github-bot pushed a commit that referenced this issue Jul 9, 2024
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this issue Jul 12, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: #53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this issue Jul 12, 2024
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this issue Jul 16, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: #53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this issue Jul 16, 2024
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: #53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #53755
Fixes: #53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit to targos/node that referenced this issue Jul 17, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: nodejs#53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
PR-URL: nodejs#53755
Fixes: nodejs#53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
targos pushed a commit to targos/node that referenced this issue Jul 17, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: nodejs#53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
PR-URL: nodejs#53755
Fixes: nodejs#53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this issue Jul 18, 2024
Original commit message:

    [arraybuffers] initialize max byte length of empty array buffers

    Without initializing the max byte length field, any empty array
    buffer captured in the snapshot can make the snapshot unreproducible.

    Refs: nodejs#53579

    Change-Id: I2489ab2e57ecbb405ec431a87d0acc92822b777c
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5662576
    Reviewed-by: Marja Hölttä <marja@chromium.org>
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#94754}

Refs: v8/v8@e061cf9
PR-URL: nodejs#53755
Fixes: nodejs#53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ehsankhfr pushed a commit to ehsankhfr/node that referenced this issue Jul 18, 2024
Original commit message:

    [rab/gsab] Remove --harmony-rab-gsab (has been on by default for a while)

    Bug: v8:11111
    Change-Id: Ie74e7737f3e2e8730820cf00f1cbc7ae02b515af
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5516580
    Commit-Queue: Marja Hölttä <marja@chromium.org>
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#93848}

Refs: v8/v8@9ebca66
PR-URL: nodejs#53522
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#53755
Fixes: nodejs#53579
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repro-exists Issues with reproductions. snapshot Issues and PRs related to the startup snapshot test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants