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

fix(xsnap): update to latest XS release #6011

Closed
wants to merge 2 commits into from
Closed

Conversation

warner
Copy link
Member

@warner warner commented Aug 19, 2022

This provides the upstream version of the snapshot-write memory leak
fix, along with other bugfixes.

closes #5975

@warner warner added the xsnap the XS execution tool label Aug 19, 2022
@warner warner self-assigned this Aug 19, 2022
@warner
Copy link
Member Author

warner commented Aug 19, 2022

Oh right, I need to update the xsnap metering ID, and also the snapshots

@warner
Copy link
Member Author

warner commented Aug 19, 2022

I made a pass through all the XS xs/sources/ changes.. I see our fix for #5975 , a lot of Module/Compartment changes, and some fuzzing-driven improvements. As usual I'm not qualified to inspect it deeply, but I don't see any obvious problems.

@dckc
Copy link
Member

dckc commented Aug 22, 2022

finding the xs/sources changes was kind of a pain. github wants to tell me how many zillion files changed in the submodule, but not what the before and after versions are!

It eventually relented. For reference:

diff --git a/packages/xsnap/moddable b/packages/xsnap/moddable
index 8d81086236..ae5ed20a96 160000
--- a/packages/xsnap/moddable
+++ b/packages/xsnap/moddable
@@ -1 +1 @@
-Subproject commit 8d810862361b426471dc422c245fa87b8633acaa
+Subproject commit ae5ed20a963b22646f2d512101dff526006ae49d
moddable/xs$ git diff --stat 8d81086236..ae5ed20a963b .
 xs/makefiles/win/xst.mak  |    1 +
 xs/platforms/esp/xsHost.c |   42 +++
 xs/platforms/esp/xsHost.h |    9 +
 xs/platforms/lin_xs.c     |   16 +-
 xs/platforms/mac_xs.c     |   16 +-
 xs/platforms/win_xs.c     |   16 +-
 xs/platforms/xsPlatform.h |    7 +
 xs/sources/xsAPI.c        |   24 +-
 xs/sources/xsAll.h        |   36 ++-
 xs/sources/xsArray.c      |   10 +-
 xs/sources/xsCode.c       |   17 +-
 xs/sources/xsCommon.c     |   11 +-
 xs/sources/xsCommon.h     |   19 +-
 xs/sources/xsDebug.c      |   46 ++-
 xs/sources/xsError.c      |   21 +-
 xs/sources/xsFunction.c   |    2 +-
 xs/sources/xsJSON.c       |    2 +
 xs/sources/xsLexical.c    |    5 +-
 xs/sources/xsLockdown.c   |   39 ++-
 xs/sources/xsMapSet.c     |   20 +-
 xs/sources/xsMemory.c     |   26 +-
 xs/sources/xsModule.c     | 2105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------
 xs/sources/xsPlatforms.c  |    8 +-
 xs/sources/xsRun.c        |   28 +-
 xs/sources/xsSnapshot.c   |   22 +-
 xs/sources/xsSourceMap.c  |   20 +-
 xs/sources/xsSyntaxical.c |    3 +
 xs/sources/xsType.c       |   30 +-
 xs/tools/xsl.c            |    3 +
 xs/tools/xslSlot.c        |   11 +-
 xs/tools/xslStrip.c       |   18 +-
 xs/tools/xst.c            |   22 +-
 32 files changed, 1566 insertions(+), 1089 deletions(-)

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of upstream changes. I'm glad we have fuzzing!

@warner warner added the automerge:rebase Automatically rebase updates, then merge label Aug 22, 2022
warner added 2 commits August 22, 2022 15:37
This provides the upstream version of the snapshot-write memory leak
fix, along with other bugfixes.

closes #5975
@warner warner removed the automerge:rebase Automatically rebase updates, then merge label Aug 22, 2022
@mhofman
Copy link
Member

mhofman commented Aug 25, 2022

Just re-ran the deployment test to see if we can trigger the same failure.

@mhofman
Copy link
Member

mhofman commented Aug 25, 2022

Ok this is reproducible in a second run! I'm getting very confused. Will try to repro locally to see if the CI environment has something to do with this.

@mhofman
Copy link
Member

mhofman commented Aug 25, 2022

Interestingly on my machine it didn't fail on the second validator node but it failed on the follower node used by the loadgen.

--- validator0-xsnap-trace/v19/00720-command.dat        2022-08-25 18:46:58.000000000 +0000
+++ chain-stage-0-xsnap-trace/v19/00720-command.dat     2022-08-25 18:49:43.000000000 +0000
@@ -1 +1 @@
-["syscall",["send","o-64",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+21"}]]
+["syscall",["send","o-69",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+21"}]]
--- validator0-xsnap-trace/v19/00732-command.dat        2022-08-25 18:46:58.000000000 +0000
+++ chain-stage-0-xsnap-trace/v19/00732-command.dat     2022-08-25 18:49:43.000000000 +0000
@@ -1 +1 @@
-["syscall",["send","o-60",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+24"}]]
+["syscall",["send","o-64",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+24"}]]
--- validator0-xsnap-trace/v19/00744-command.dat        2022-08-25 18:46:58.000000000 +0000
+++ chain-stage-0-xsnap-trace/v19/00744-command.dat     2022-08-25 18:49:43.000000000 +0000
@@ -1 +1 @@
-["syscall",["send","o-69",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+27"}]]
+["syscall",["send","o-60",{"methargs":{"body":"[\"makeCollectFeesInvitation\",[]]","slots":[]},"result":"p+27"}]]

@mhofman
Copy link
Member

mhofman commented Jan 6, 2023

Closing this PR and tracking work to update to newer SDK in #6759

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

Successfully merging this pull request may close these issues.

xsnap leaks memory during snapshot write
3 participants