Closed C-4PO closed 3 years ago
Yes, this is bad!!!
Basically running the following command the second time clears the block:
sudo /Applications/SelfControl.app/Contents/MacOS/org.eyebeam.SelfControl $(id -u $(whoami)) --install
Output:
2017-11-28 11:20:45.210 org.eyebeam.SelfControl[16243:65649299] Using your own bundle identifier as an NSUserDefaults suite name does not make sense and will not work. Break on _NSUserDefaults_Log_Nonsensical_Suites to find this
2017-11-28 11:20:45.222 org.eyebeam.SelfControl[16243:65649299] ERROR: Lock already established. Attempting to stop block.
pfctl: Use of -f option, could result in flushing of rules
present in the main ruleset added by the system at startup.
See /etc/pf.conf for further details.
No ALTQ support in kernel
ALTQ related functions disabled
pfctl: pf not enabled
2017-11-28 11:20:45.292 org.eyebeam.SelfControl[16243:65649299] INFO: Block successfully cleared.
2017-11-28 11:20:46.370 org.eyebeam.SelfControl[16243:65649299] INFO: Block cleared.
/Library/LaunchDaemons/org.eyebeam.SelfControl.plist: Could not find specified service
-219
Stop doing that!
If I remove this line from the code, this should remove this bug (where if you run the install command twice it clears all blocks): https://github.com/SelfControlApp/selfcontrol/blob/c50bdfce48aa43a3b2c5ce91dcb2b6246bf75532/HelperMain.m#L201
@cstigler in the comment above this section I see "This should definitely not be happening though" lol...does this little section have a purpose? Is it ok to remove the block clearing here?
Hi @mhfowler! Thanks for looking into this :)
The point of that section is to avoid breaking the app in case of a bug or unexpected configuration.
Basically, with any unexpected cases that pop up in SelfControl's blocking procedure, we have two choices: 1) proceed with the aim of keeping the block on ("fail-blocked"), or 2) proceed with the aim of making sure the block will come off ("fail-safe"). In this case, we have an unexpected case from the app perspective, and I decided to fail-safe in case the existence of two blocks at the same time indicates some sort of system reliability issue.
We definitely could get rid of that line (with a bunch of testing), but it strikes me that more correct behavior would actually be removing lines 203-204 instead so that the installation continues after removing the old block. I think this was the intended behavior when I wrote it, based on the comment "we try to remove a block and continue as normal". The point was to remove the old block and then re-add the new block, so we still end up with a block running but it's the new one, not whatever was already there. I haven't tested this; but thoughts on that plan?
Thanks for the explanation - that makes sense to me. I'd be happy to submit a PR with this lil change, but you probably know how to test everything anyway, but thanks so much for responding.
I've been enjoying using SelfControl, and have consciously not been "looking for" a way around it, but I was just trying to setup a recurring block using the scripting tutorial and stumbled upon this way around it -- so I'm looking forward to not knowing a way around it again lol.
Also, I guess if I was playing role of code reviewer here, I would also get rid of the log statement above that says its an error if a block already exists -- in this case it seems less like an error, and more like you just want "Installing A Block" to be idempotent (and remove any block that already existed).
Last thing.. I like your fail-blocked and fail-safe terminology.... it would be interesting if this was a configurable flag (which way you wanted to fail)... but also for most users probably unnecessary
Ah wait... if you keep the "remove block line" here.... that still leaves you with a way around a block... since lets say you had a block of 5 hours set... you could install a new block of 1 minute.... and as I understand your plan it would remove the old block and add the new shorter one.... so I would vote for "fail-block" in this case.... or some way of making it "fail-block" if you see that the cause is a user running an --install command .... because really personally what I want is to not be able to remove an extant block by running the --install command
I've just tried building SelfControl locally so I could test getting rid of the removeBlock line... but running into build issues... is there a separate place where there is documentation on getting the project to run in xcode?
@mhfowler re build docs: unfortunately, no specifics docs on building in Xcode. Shouldn't be anything too non-standard though. Did you install the CocoaPods?
It is true that keeping the removeBlock()
call will retain a possible way to get around the block (by replacing your block with a very short one), but I'm not sure it's worth any risk to fix that. While I don't want to ruin the app's unblockability for you (and I hate to encourage you to Google), there are much, much easier ways to get around the block which have been discovered by many more users. Until we've gotten rid of the gaping holes in the bucket, I don't think we need to spend too much getting rid of the tiny leak!
That's fair enough -- I wanted to fix this particular way around because this way around appears by simply following the tutorial listed in the documentation for SelfControl... to me there is some difference between a way around SelfControl which you've sought out via googling... and one which just appears by simply following the documentation of SelfControl
but can understand why you would have other concerns.
and I did install the CocoaPods, so not sure what's wrong with my build.. I can poke around
Hmm, I hear you on that. But given that the vast majority of users only use the GUI app, I'm less concerned with workarounds that require command-line use (the main way users find does not).
That said, definitely would be into the fix that keeps the removeBlock()
call , which should still fix the case of you noticing it while you walked through the tutorial (i.e. it'd be much less discoverable). I'm uncomfortable with removing that section, at least all at once - even if it's a very low risk of increasing the number of perma-blocks, it doesn't seem worth it to me.
And - if you do figure out what the build issues are and they seem like they might affect other people, please do commit a fix or add it to a README! Future generations will thank you :)
This is fixed in the latest version of the code, which will be released with SelfControl 4.0 in a month-ish.
Running the init command twice in terminal undo's the block. It's not good that I found this out.