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

std::bad_alloc - rework file storage from internal buffer to stream #357

Closed
ilg-ul opened this issue Aug 24, 2023 · 8 comments
Closed

std::bad_alloc - rework file storage from internal buffer to stream #357

ilg-ul opened this issue Aug 24, 2023 · 8 comments

Comments

@ilg-ul
Copy link

ilg-ul commented Aug 24, 2023

Describe the bug

The GitHub Actions runner failed with std::bad_alloc when trying to publish a large archive file.

To Reproduce

Steps to reproduce the behavior:

  • I don't know if this is useful, but try to publish a large (506MB) archive from a machine with 4 GB of RAM

Expected behavior

I successfully used the ncipollo/release-action@v1 action to publish many releases, without any problems, and I expected to run properly for large archives too.

Desktop (please complete the following information):

  • OS: Ubuntu 18 under Docker, on a Raspberry Pi OS 32-bit

Additional context

From the downloaded log:

2023-08-24T07:21:21.6209697Z ##[group]Run ncipollo/release-action@v1
2023-08-24T07:21:21.6212311Z with:
2023-08-24T07:21:21.6214242Z   allowUpdates: true
2023-08-24T07:21:21.6216227Z   artifacts: build/linux-arm/deploy/*
2023-08-24T07:21:21.6219179Z   bodyFile: .github/workflows/body-github-pre-releases-test.md
2023-08-24T07:21:21.6221631Z   commit: master
2023-08-24T07:21:21.6223424Z   draft: false
2023-08-24T07:21:21.6225352Z   name: Test binaries
2023-08-24T07:21:21.6227421Z   omitBodyDuringUpdate: true
2023-08-24T07:21:21.6229772Z   omitDraftDuringUpdate: true
2023-08-24T07:21:21.6231950Z   omitNameDuringUpdate: true
2023-08-24T07:21:21.6234193Z   owner: xpack-dev-tools
2023-08-24T07:21:21.6236072Z   prerelease: true
2023-08-24T07:21:21.6237887Z   replacesArtifacts: true
2023-08-24T07:21:21.6239596Z   repo: pre-releases
2023-08-24T07:21:21.6241267Z   tag: test
2023-08-24T07:21:21.6243990Z   token: ***
2023-08-24T07:21:21.6279488Z   generateReleaseNotes: false
2023-08-24T07:21:21.6281594Z   makeLatest: legacy
2023-08-24T07:21:21.6283495Z   omitBody: false
2023-08-24T07:21:21.6285071Z   omitName: false
2023-08-24T07:21:21.6286917Z   omitPrereleaseDuringUpdate: false
2023-08-24T07:21:21.6288837Z   removeArtifacts: false
2023-08-24T07:21:21.6290625Z   skipIfReleaseExists: false
2023-08-24T07:21:21.6292522Z   updateOnlyUnreleased: false
2023-08-24T07:21:21.6294163Z ##[endgroup]
2023-08-24T07:21:21.7169857Z ##[command]/usr/bin/docker exec  05bf2dba69b738d2cc16bd0b648643d0552e695ec56bf215e681aea05f4b1fde sh -c "cat /etc/*release | grep ^ID"
2023-08-24T07:21:29.2705233Z terminate called after throwing an instance of 'std::bad_alloc'
2023-08-24T07:21:29.2706614Z   what():  std::bad_alloc

The workflow file is:

The run is:

I don't know which process threw this exception, if it is related to this GitHub Action or not, but it looks like a C++ out of memory condition.

Does this Action try to move the file by copying it into memory? Since the machine I'm running this is a small Raspberry Pi, with only 4 GB of memory.

If this is true, probably the code should be reworked to use a stream, which uses less memory.

To be noted that I successfully published even larger files (549 MB), but on machines with more memory.

@ilg-ul
Copy link
Author

ilg-ul commented Aug 24, 2023

I found a core file in the project folder, and gdb reported:

Core was generated by `/__e/node16/bin/node /__w/_actions/ncipollo/release-action/v1/dist/index.js'.
Program terminated with signal SIGABRT, Aborted.
#0  0xb6bed306 in ?? ()

So it looks like node ran out of memory while running the Action.

@ilg-ul ilg-ul changed the title std::bad_alloc std::bad_alloc - rework file storage from internal buffer to file Aug 24, 2023
@ilg-ul ilg-ul changed the title std::bad_alloc - rework file storage from internal buffer to file std::bad_alloc - rework file storage from internal buffer to stream Aug 24, 2023
@ilg-ul
Copy link
Author

ilg-ul commented Aug 24, 2023

I took a look at the code, and in Artifacts.ts I noticed:

    readFile(): Buffer {
        return readFileSync(this.path)
    }

This might be acceptable for small files and runners with lots of memory, but it would be safer to use a stream.

@ncipollo
Copy link
Owner

The readFile method on Artifact now returns a ReadStream. It does look like octokit supports this as an argument type for the body: https://github.com/octokit/request.js/blob/main/test/request.test.ts#L906. I verified in my test repos that artifacts still upload with the stream.

Now whether or not they actually handle the stream correctly internally is another question ;)

@ilg-ul
Copy link
Author

ilg-ul commented Aug 24, 2023

Thank you for the quick fix, I appreciate it.

Tomorrow I'll probably have another such large artifact to upload, and I'll confirm if it was successful.

@ilg-ul
Copy link
Author

ilg-ul commented Aug 24, 2023

BTW, do you know if the self-hosted runner will download the new version of your action or will cache and re-use the same version that crashed before? Since the GitHub runner is active continuously, the machine runs non-stop. Probably you should have increased the version in package.json.

@ncipollo
Copy link
Owner

Have you seen any documentation about that version being relevant? I was under the impression actions just utilized tags, branches and commits.

Not actually sure how actions would be cached on a runner. I would assume they would see that the tag points to a new commit and pull it down, if not clone the action every run (but I don't really know for sure).

@ilg-ul
Copy link
Author

ilg-ul commented Aug 24, 2023

Have you seen any documentation about that version being relevant?

Unfortunately not... I extrapolated from my experience with the modules used by npm, which I also use in xpm. But the runner is open source, perhaps this detail can be understood from the code.

The build will take 10 more hours and tomorrow we'll know if it worked...

@ilg-ul
Copy link
Author

ilg-ul commented Aug 25, 2023

The large build from yesterday (550 MB) was the first build based on gcc 13.2.

This night build was an update based on gcc 12.3, and, being older, was slightly smaller, only 452 MB, so I can not fully confirm, but let's assume that the problem was solved. We'll know better when 13.3 will be out.

Thank you!

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