fix(mac): Workaround for hdiutil randomly failing (#5431) #5464

chokladmal · 2020-12-10T15:22:17Z

For some but not all Mac users building dmg files often fails.

After some investigations the cause seems to be that hdiutil create (when being run to create a temporary dmg file in createStageDmg) fail when trying to unmount the created volume due to "resource busy". As this only affects some Macs it seems to be caused by something on those machines. Suggested is antivirus software but that's not confirmed.

It turns out that after retrying the hdiutil create command a few times it usually succeeds and this workaround is based on that idea - to simply retry the command up to five times before throwing and failing the build. This does not fix the real issue!

Tracking issue: #5431

…5431) For unknown reasons hdiutil fails randomly when trying to create the dmg in the tmp location. This workaround simply retries until it works or throws the latest error if it fails ten times.

mmaietta

I'm not sure on what could be the actual solution here, as the random failures seem to be more OS-related from what I've searched and my personal experience.

As an improvement here, I'd recommend using builder-util retry<T> function.
https://github.com/electron-userland/electron-builder/blob/master/packages/builder-util/src/util.ts#L384
Just need to export it and it simplifies all your usage to 2 lines.

Related notes: This may be misusing the logger's debug flag, but you could also conditionally add -debug since the retry function doesn't have any explicit logging info. I also increased duration to 1s as sometimes I had to wait that long before a retry when I had my own hdiutil bash scripts

if (log.isDebugEnabled) {
  imageArgs.push("-debug")
}
await retry(spawn("hdiutil", imageArgs), 5, 1000)
return tempDmg

BelinChung · 2020-12-16T12:10:04Z

It seems to be the problem caused by this line

// imageArgs.push("-fs", "HFS+", "-fsargs", "-c c=64,a=16,e=16")

When I commented it out, the dmg package was successfully built on Big Sur.

mmaietta · 2020-12-16T15:26:05Z

Out of curiosity, can you try just omitting the -fsargs < >? I'm wondering if all we need is

imageArgs.push("-fs", "HFS+")

From the man:

-fsargs newfs_args
                          additional arguments to pass to whatever newfs pro-
                          gram is implied by -fs.  newfs_hfs(8) has a number
                          of options that can reduce the amount of space
                          needed by the filesystem's data structures.  Sup-
                          pressing the journal with -fs HFS+ and passing argu-
                          ments such as -c c=64,a=16,e=16 to -fsargs will min-
                          imize gaps at the front of the filesystem, allowing
                          resize to squeeze more space from the filesystem.
                          For truly optimal filesystems, use makehybrid.

chokladmal · 2020-12-17T16:29:28Z

Thank you for your comments! I really appreciate that you're helping out with this :)
As I mentioned in the issue thread I've tried running hdiutils with -attach with some success. It doesn't fail but it will have to be unmounted at a later stage instead. Right now I have somewhat limited time but I will keep trying, especially the suggested changes to the args.

lutzroeder · 2020-12-17T22:38:13Z

@chokladmal

Is there a linked issue with repro steps this change is tracking?
What version of electron-builder and macOS are you running and does the Terminal have full disk access?
Did you track down the actual error by running with export DEBUG_DMG=true and repro by running the hdiutil command in isolation?

Trying to find ways to move this from "no idea why this is happening but a for loop might hide it" towards having a reliable repro, diagnosing the underlying issue and driving a fix.

chokladmal · 2020-12-18T00:37:58Z

@lutzroeder I saw that you had already commented in the issue but for the record it's this one #5431

I'm running electron-builder 22.9.1 and macOS Catalina 10.15.17

In #5431 we've narrowed it down to hdiutil failing due to "resource busy" when hdiutil create tries to unmount the volume.

chokladmal · 2020-12-18T00:40:43Z

Out of curiosity, can you try just omitting the -fsargs < >? I'm wondering if all we need is

imageArgs.push("-fs", "HFS+")

From the man:

-fsargs newfs_args
                          additional arguments to pass to whatever newfs pro-
                          gram is implied by -fs.  newfs_hfs(8) has a number
                          of options that can reduce the amount of space
                          needed by the filesystem's data structures.  Sup-
                          pressing the journal with -fs HFS+ and passing argu-
                          ments such as -c c=64,a=16,e=16 to -fsargs will min-
                          imize gaps at the front of the filesystem, allowing
                          resize to squeeze more space from the filesystem.
                          For truly optimal filesystems, use makehybrid.

I've tried it. At first it seemed to be working but it finally failed with the same


DIHLDiskImageCreate() returned 49168
hdiutil: create failed - Resource busy

after ten or so tries.

Instead of having a similar implementation in createStageDmg as the already existing "retry" function in builder-util it is now exported from that package and used to handle the rerun-on-failure in createStageDmg.

…iutil-failing

mmaietta · 2020-12-22T23:45:29Z

This should be fixed (from a PR revert) in the latest prerelease cut 22.10.4

lutzroeder · 2020-12-23T01:03:10Z

@mmaietta #5431 might be a different issue? The reverted PR was a recent regression in 22.10.3 which isn't broadly used and it repros reliably. This issue sounds like it succeeds on retry, shows a different error and also happened on previous versions? Next steps are probably either finding a reliable repro and tracking down why this is happening.

chokladmal · 2020-12-23T09:41:04Z

If it is antivirus or something else maybe having something imitating that behaviour? A script opening files on the volume as soon as it's mounted and files are added could perhaps trigger the problem. I can try it out once I'm back at my company laptop after the holidays.

lutzroeder · 2020-12-23T18:59:48Z

@chokladmal, the goal is to root cause the issue, less how to recreate behavior that would require adding a retry approach to the build pipeline. Masking fragile behavior that isn't well understood tends to lead to more fragile and unpredictable behavior. For example, if this is caused by a virus scanner, the right fix is probably to suspend the scanner during build, add an exclusion or work with the vendor. If the issue is a bug in hdiutil the right fix would be fixing hdiutil. Retry should be more of a last resort if the underlying issue is understood and the real fix is blocked. Even then it should probably be opt-in based on a flag.

shrpne · 2021-01-12T14:39:19Z

@lutzroeder can it be released as a workaround to fix current issue and then be replaced with real fix, when it ready?

Because for now there is no workaround. I even can't install chokladmal's fork because it is monorepo. And since it is typescript it requires a lot of tooling to apply his patch to my own project.

I see no problems with releasing it. It should not cause performance issues or bugs to other users. The only problem with it, it is not beautiful, but it works.

mmaietta · 2021-01-12T15:21:55Z

@shrpne here's a script that @lutzroeder put together for compiling any branch and patching the results. Run from your current app directory (with electron-builder one dir up. You can adjust paths as needed)

#!/bin/bash

pushd ../electron-builder
yarn compile
popd

rsync --recursive ../electron-builder/packages/app-builder-lib/out/ ./node_modules/app-builder-lib/out/
rsync --recursive ../electron-builder/packages/app-builder-lib/electron-osx-sign/ ./node_modules/app-builder-lib/electron-osx-sign/
rsync --recursive ../electron-builder/packages/builder-util/out/ ./node_modules/builder-util/out/
rsync --recursive ../electron-builder/packages/dmg-builder/out/ ./node_modules/dmg-builder/out/

cp ../electron-builder/packages/app-builder-lib/scheme.json ./node_modules/app-builder-lib/scheme.json

yarn patch-package app-builder-lib
yarn patch-package builder-util
yarn patch-package dmg-builder

lutzroeder · 2021-01-12T19:20:30Z

@shrpne not my call to make. This is up to the maintainer. This is an open source project so the challenge isn't shipping workarounds. The question is more who would own getting to the bottom of this and drive the right fix.

lutzroeder · 2021-01-12T19:22:05Z

@mmaietta the script looks like it got merged from different sources. Why is patch-package needed?

mmaietta · 2021-01-12T20:55:45Z

@lutzroeder patch-package allows changes to be persisted in the project repo as a .patch file to save your local changes.

shrpne · 2021-01-13T08:12:07Z

@mmaietta thank you for sharing this script, but it still looks pretty complicated, as I understand it is can't be used as a drop-in solution.

@lutzroeder honestly I don't understand, why opensource project can't accept temporary workaround, while waiting for real fix, which then will replace this workaround.
From my point of view, working opensource project with temporary workaround is better than temporary not working opensource project without workarounds.

lutzroeder · 2021-01-13T08:58:16Z

@shrpne sure, not arguing that and it’s not my call to make for this repo. You have to ask the maintainer for a release. Why not come up with the real fix now. Are you signing up to get to the bottom of this later. What is going to change to make this easier when folks are hitting the issue less often or in more random circumstances due to the workaround masking the real issue. I’m more asking why the problem isn’t better understood and what can be done about that. You are asking for the cheapest way to make it not your problem.

jzybert · 2021-03-02T18:31:57Z

Which version of electron will this be included in?

mmaietta · 2021-03-02T21:57:45Z

Please update to latest electron-builder@22.10.5

mmaietta requested changes Dec 14, 2020

View reviewed changes

eikindred added 2 commits December 18, 2020 02:02

Merge remote-tracking branch 'upstream/master' into workaround-for-hd…

31a6de2

…iutil-failing

develar merged commit 53270cf into electron-userland:master Jan 15, 2021

This was referenced Mar 7, 2021

Bump electron-builder from 22.9.1 to 22.10.5 in /electron wireapp/wire-webapp#10666

Merged

Bump electron-builder from 22.9.1 to 22.10.5 Threor/soundboard-frontend#23

Merged

fix(mac): Workaround for hdiutil randomly failing (#5431) #5464

fix(mac): Workaround for hdiutil randomly failing (#5431) #5464

Conversation

chokladmal commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmaietta left a comment

Choose a reason for hiding this comment

Uh oh!

BelinChung commented Dec 16, 2020

Uh oh!

mmaietta commented Dec 16, 2020

Uh oh!

chokladmal commented Dec 17, 2020

Uh oh!

lutzroeder commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chokladmal commented Dec 18, 2020

Uh oh!

chokladmal commented Dec 18, 2020

Uh oh!

mmaietta commented Dec 22, 2020

Uh oh!

lutzroeder commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chokladmal commented Dec 23, 2020 via email

Uh oh!

lutzroeder commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shrpne commented Jan 12, 2021

Uh oh!

mmaietta commented Jan 12, 2021

Uh oh!

lutzroeder commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lutzroeder commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmaietta commented Jan 12, 2021

Uh oh!

shrpne commented Jan 13, 2021

Uh oh!

lutzroeder commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jzybert commented Mar 2, 2021

Uh oh!

mmaietta commented Mar 2, 2021

Uh oh!

chokladmal commented Dec 10, 2020 •

edited

Loading

lutzroeder commented Dec 17, 2020 •

edited

Loading

lutzroeder commented Dec 23, 2020 •

edited

Loading

lutzroeder commented Dec 23, 2020 •

edited

Loading

lutzroeder commented Jan 12, 2021 •

edited

Loading

lutzroeder commented Jan 12, 2021 •

edited

Loading

lutzroeder commented Jan 13, 2021 •

edited

Loading