gui: Prevent idle sleep in macos while in IBD

promag commented at 2:27 pm on June 4, 2019: member

Fix #16125

Reference Technical Q&A QA1340 Listing 2.

refactor: Simplify CAppNapInhibitor interface 99547c5d52

gui: Prevent idle sleep if in macos while in IBD 6ad4ad2869

promag force-pushed on Jun 4, 2019

fanquake added the label GUI on Jun 4, 2019

fanquake added the label macOS on Jun 4, 2019

in src/qt/bitcoingui.cpp:928 in 99547c5d52 outdated

924@@ -925,7 +925,7 @@ void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVer
925 {
926 // Disabling macOS App Nap on initial sync, disk and reindex operations.
927 #ifdef Q_OS_MAC
928-    (m_node.isInitialBlockDownload() || m_node.getReindex() || m_node.getImporting()) ? m_app_nap_inhibitor->disableAppNap() : m_app_nap_inhibitor->enableAppNap();
929+    m_app_nap_inhibitor->setAppNapEnabled(!m_node.isInitialBlockDownload());

promag commented at 2:28 pm on June 4, 2019:

Note that isInitialBlockDownload() already checks getReindex and getImporting.

promag commented at 2:29 pm on June 4, 2019:

Also, say goodbye to this ternary.

fanquake commented at 8:59 am on August 21, 2019:

Don’t you need to drop the ! here? Otherwise it looks like AppNap and IdleSleepPrevention will actually be disabled during IBD.

DrahtBot commented at 4:20 pm on June 4, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#18152 (qt: Use NotificationStatus enum for signals to GUI by hebasto)
#16367 (Multiprocess build support by ryanofsky)
#10102 ([experimental] Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

promag renamed this:
~~gui: Prevent idle sleep if in macos while in IBD~~
gui: Prevent idle sleep in macos while in IBD
on Jun 6, 2019

Empact commented at 2:11 am on June 10, 2019: member

Concept ACK

Sjors commented at 2:36 am on June 26, 2019: member

Concept ACK, but I’m having difficulty reliably producing this stuff. So let’s make sure this actually works as advertised.

From the Apple Doc, it’s important to note that this does not impact “forced sleep”:

Forced sleep occurs when the user takes some sort of direct action to cause the machine to sleep. Closing the lid on a laptop or selecting sleep from the Apple menu both cause forced sleep. The system will also induce forced sleep under certain conditions, for example, a thermal emergency or a low battery. Idle sleep occurs when the machine is unused for a specific period of time configured in the Energy Saver System Preferences.

Some hints for testing:

use pmset -g and pmgset -g log (background info)
keep an eye on (sleep prevented by sharingd), you may want to turn off BlueTooth (and ActivityMontor?)
the network tab can be deceptive, the log file shows gaps more clearly
disksleep (e.g. 10 minutes) is probably the most relevant setting
I’m not sure how tcpkeepalive (e.g. 1 minute) plays into things, IBD usually isn’t idle that long
you can configure a hot corner to turn your display off

I tested in battery mode and set disksleep to 1 minute. I started QT without the PR and let it sync a little bit on a fresh datadir. I moved the app as well as the log Console to the background. I then used the hot corner to turn off the display, and waited 5 minutes. This results in a 4 minute gap in the log, as expected.

In other other attempts there was no gap, so maybe it was because I didn’t have QT / Console in the background, or something else on my system was interfering.

I then ran with this PR. I don’t see a “sleep prevented by” entry in pmset -g, but that’s perhaps expected. I again turned off the display, and there was a gap.

Perhaps the hot corner counts as “forced sleep”. But if I just let the screen go idle, I also get a gap in the log.

Notably I don’t see “Downloading blockchain” or even “bitcoin-qt” in the pmset -g log log.

fanquake commented at 9:08 am on August 21, 2019: member

I’m Concept NACK on this.

The original issue states:

There’s another problem however, which is that by default MacBooks suspend when the display is off, even when plugged into a power socket. This causes IBD to pause, which is annoying if you’re trying to sync at night.

Do we need to link a new framework and add more Objective-C code to Bitcoin Core to fix what is essentially (non-technical?) users not installing something like KeepingYouAwake or Amphetamine (or doing any other form of power/sleep management etc) to stop their screen from dimming during IBD?

I’d rather not introduce more OS specific code (power management or in general) into bitcoin-qt.

From the Apple Doc, it’s important to note that this does not impact “forced sleep”

I’d hope it doesn’t. If Bitcoin-Core could somehow keep your computer running after you’ve told it to sleep that’s just about malware. Even stopping the machine from idling while the screen is dimmed could potentially lead to complaints about excess power usage etc.

FWIW 6ad4ad286963e06abab7b6c86dda28aa140f643c doesn’t currently compile for me:

 0  CXXLD    test/test_bitcoin
 1  AR       qt/libbitcoinqt.a
 2  CXXLD    qt/bitcoin-qt
 3  CXXLD    qt/test/test_bitcoin-qt
 4Undefined symbols for architecture x86_64:
 5  "_IOPMAssertionCreateWithName", referenced from:
 6      CAppNapInhibitor::setAppNapEnabled(bool) in libbitcoinqt.a(libbitcoinqt_a-macos_appnap.o)
 7  "_IOPMAssertionRelease", referenced from:
 8      CAppNapInhibitor::setAppNapEnabled(bool) in libbitcoinqt.a(libbitcoinqt_a-macos_appnap.o)
 9ld: symbol(s) not found for architecture x86_64
10clang: error: linker command failed with exit code 1 (use -v to see invocation)

However if I configure using depends and fix the isInitialBlockDownload condition I can see (when running pmset -g log):

 02019-08-21 16:54:38 +0800 : Showing all currently held IOKit power assertions
 1Assertion status system-wide:
 2   BackgroundTask                 0
 3   ApplePushServiceTask           0
 4   UserIsActive                   1
 5   PreventUserIdleDisplaySleep    0
 6   PreventSystemSleep             0
 7   ExternalMedia                  0
 8   PreventUserIdleSystemSleep     1
 9   NetworkClientActive            0
10Listed by owning process:
11   pid 65924(Bitcoin-Qt): [0x0002e12f00018a8f] 00:00:07 NoIdleSleepAssertion named: "Downloading blockchain"

Sjors commented at 10:20 am on August 21, 2019: member

Do we need to link a new framework and add more Objective-C code to Bitcoin Core to fix what is essentially (non-technical?) users not installing something like KeepingYouAwake or Amphetamine (or doing any other form of power/sleep management etc) to stop their screen from dimming during IBD?

I’d rather not encourage users to install those apps. They’re not good for your computer and if it becomes a tradition then they become an attack vector. Keeping the display on is also not desirable. macOS itself performs software upgrades when the screen is off, the computer is plugged in and on wifi, exactly because that’s a good time.

promag commented at 12:11 pm on August 28, 2019: member

Thanks @fanquake, let’s get back to this after 0.19.

jonasschnelli commented at 11:42 am on October 9, 2019: contributor

To make progress here: can we learn from others? I’m pretty sure there are hundreds of macOS applications having the same problem. Render software, torrent-apps, web-servers, etc. Anything hints to a “best practice”?

I kinda agree with @fanquake that adding more native Obj-C is bad. Ideally it would be on another layer (Qt or even better app config level / plist). I have hope that there is a flag on the configuration layer that tells the OS “this app runs in background and doesn’t like to be set to sleep” (like a webserver).

On top, once Core is in sync, users may expect that it goes to “sleep” when requested.

fanquake commented at 11:29 am on February 26, 2020: member

Thanks @fanquake, let’s get back to this after 0.19.

I’m still not convinced this is a good idea. I don’t think GUI only, OS specific power management functionality, written in Objective-C, is something we should have to maintain in this repository.

promag commented at 11:31 am on February 26, 2020: member

Agree.

promag closed this on Feb 26, 2020

promag deleted the branch on Feb 26, 2020

Sjors commented at 11:54 am on February 26, 2020: member

I don’t think GUI only, OS specific power management functionality, written in Objective-C, is something we should have to maintain in this repository.

I’m not opposed to that as long as it’s simple enough and improves the users first experience, but in this case it’s perhaps too confusing.

DrahtBot locked this on Feb 15, 2022

gui: Prevent idle sleep in macos while in IBD #16145

Conflicts