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: backport 3b21b6d from v8 to fix profiler crash #28531

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

indutny
Copy link
Contributor

@indutny indutny commented Apr 6, 2021

Description of Change

Fix: #28472

Release Notes

Notes: Fix crash when using profiler in devtools.

@ckerr
Copy link
Member

ckerr commented Apr 6, 2021

@indutny, looks like this patch needs another look

Applying: Allow empty source URL for asm modules
Using index info to reconstruct a base tree...
M	src/profiler/profiler-listener.cc
Falling back to patching base and 3-way merge...
Auto-merging src/profiler/profiler-listener.cc
CONFLICT (content): Merge conflict in src/profiler/profiler-listener.cc
error: Failed to merge in the changes.
Patch failed at 0010 Allow empty source URL for asm modules
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Traceback (most recent call last):
  File "src/electron/script/apply_all_patches.py", line 34, in <module>
    main()
  File "src/electron/script/apply_all_patches.py", line 30, in main
    apply_patches(json.load(config_json))
  File "src/electron/script/apply_all_patches.py", line 16, in apply_patches
    committer_name="Electron Scripts", committer_email="scripts@electron")
  File "/home/builduser/project/src/electron/script/lib/git.py", line 107, in import_patches
    am(repo=repo, **kwargs)
  File "/home/builduser/project/src/electron/script/lib/git.py", line 77, in am
    proc.returncode))
RuntimeError: Command ['git', '-C', 'src/v8', '-c', 'user.name=Electron Scripts', '-c', 'user.email=scripts@electron', '-c', 'commit.gpgsign=false', 'am', '--3way', '--keep-cr'] returned 128
Error: Command 'python3 src/electron/script/apply_all_patches.py src/electron/patches/config.json' returned non-zero exit status 1 in /home/builduser/project
Hook 'python3 src/electron/script/apply_all_patches.py src/electron/patches/config.json' took 52.25 secs


Exited with code exit status 2
CircleCI received exit code 2

@indutny
Copy link
Contributor Author

indutny commented Apr 6, 2021

@ckerr ah, this is because the master branch uses V8 9.1.127 that is already fixed. I'll re-target this PR to 12-x-y. Sorry for the noise!

@indutny indutny requested review from a team as code owners April 6, 2021 05:13
@indutny indutny changed the base branch from master to 12-x-y April 6, 2021 05:13
@indutny
Copy link
Contributor Author

indutny commented Apr 6, 2021

@ckerr should be better now. PTAL

@codebytere
Copy link
Member

@indutny this patch needs to be fixed up - the commit to push to fix it is here

@indutny
Copy link
Contributor Author

indutny commented Apr 6, 2021

Thank you @codebytere . Force pushed again.

@indutny
Copy link
Contributor Author

indutny commented Apr 7, 2021

Sorry for a ping, but it'd be great if it could get into 12-x-y before the release (which apparently wasn't cut out just yet!)

@zcbenz zcbenz changed the title backport 3b21b6d from v8 to fix profiler crash fix: backport 3b21b6d from v8 to fix profiler crash Apr 7, 2021
@zcbenz zcbenz added semver/patch backwards-compatible bug fixes backport-check-skip Skip trop's backport validity checking backport This is a backport PR labels Apr 7, 2021
@zcbenz zcbenz merged commit 1cf09e7 into electron:12-x-y Apr 7, 2021
@release-clerk
Copy link

release-clerk bot commented Apr 7, 2021

Release Notes Persisted

Fix crash when using profiler in devtools.

@indutny indutny deleted the fix/gh-28472 branch April 7, 2021 05:52
@indutny
Copy link
Contributor Author

indutny commented Apr 7, 2021

Awesome. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport This is a backport PR backport-check-skip Skip trop's backport validity checking semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DevTools make performance profile cause renderProcess crash @12.x.x
4 participants