Only some lines of tridactylrc are executed

[zsugabubus]

Ok, so... first of all sorry for opening a new issue (maybe relates to #469), but I think it deserves a separate page. This issue made me crazy.

After three days of frustration I finally shrinked down my tridactylrc to this:

sanitise tridactyllocal tridactylsync

set storageloc     local " secret feature switcher eastern egg

bind g1 tab 1
bind g3 tab 3 "*
bind g5 tab 5
bind g7 tab 7 "*
bind g2 tab 2
bind g4 tab 4 "*
bind g6 tab 6
bind g8 tab 8 "*
bind g9 tab 9

set searchurls.gh  https://github.com/search?utf8=✓&q= "*

bind J tabprev
bind K tabnext "*

I just couldn't imagine before, why my bindings aren't working for days. Guess what happens: only starred lines gets executed (i.e. every second). "Impossible." - I said. If I comment out "storageloc" line, or set it to "sync" it seems to work--however I don't use Firefox sync at all, so I don't know if it should make any difference by the way.

I was curious, so I went one step further to discover what happens to other lines.

sanitise tridactyllocal tridactylsync

set storageloc     local " That line from above.

bind g1 tab 1 "+ 1
bind g3 tab 3 "*  6
bind g5 tab 5 "+ 2
bind g7 tab 7 "*  7
bind g2 tab 2 "+ 3
bind g4 tab 4 "*  8
bind g6 tab 6 "+ 4
bind g8 tab 8 "   9
aoesnuhasoneuhbind g9 tab 9

set searchurls.gh  https://github.com/search?utf8=✓&q= "*+

bind J tabprev "    10
bind K tabnext "*+ 5

After a few hundreds of restarts here's my result. Sometimes just starred or plus signed, BUT sometimes all lines (except "g9" of course) get executed (!!!).(Numbers mean nmap order I saw from :viewconfig.)

I don't know if it makes any sense in the context of JavaScript, but as I see it must be some race condition in the background, so I feel that I should also mention that my whole Firefox profile lives in RAM.

[glacambre]

Hi, thanks for reporting this issue. It does sound like a race condition and also a lot like #1197. What Firefox and Tridactyl versions are you using?

[zsugabubus]

Mozilla Firefox 66.0.2
Tridactyl 1.14.9-1.14.9

[madduck]

I am trying to wrap my head around how this can be. Is this a function of the asynchronous code and promises? If so, would be it possible to employ a semaphore of sorts to ensure that the code is executed as a transaction?

[bovine3dom]

It must be a race condition, yes. We even added a 100ms delay between lines to try to ameliorate it.

It would be quite straightforward to set up a failing test for it as we can just include an RC file as a string -

tridactyl/src/background/config_rc.ts

Line 45 in ae40375

export async function runRc(rc: string) {

- and then check that the config is right.

I suspect the race condition is in config.set - but I'm not sure. I think #1764 was trying to do that. A lock could help :)

[bovine3dom]

#1764 has quite a lot of useful information in it. The trouble is that we are using the browser storage to synchronise configuration between tabs via an onChanged event. config.set says that it is done before the onChanged event is fired, which is a Firefox design choice we have no control over.

I think we need to stop using onChanged -

tridactyl/src/lib/config.ts

Line 1645 in ae40375

browser.storage.onChanged.addListener((changes, areaname) => {

- to synchronise configuration between tabs and create our own solution instead.

[bovine3dom]

If anyone is interested in removing onChanged, it might be easier to remove it from state.ts first as that is a far simpler object -

tridactyl/src/state.ts

Line 65 in ae40375

browser.storage.onChanged.addListener((changes, areaname) => {

[bovine3dom]

It would be nice if #2137 has fixed this. Could someone try the latest betas once this message is >10 mins old and report back?

[bovine3dom]

5% of your issues, @zsugabubus, are probably because we don't support inline comments in the RC file (yet) -

set mysetting blah " this isn't a valid comment
" this is the only kind of comment we support

in the browser, :get mysetting would return blah " this isn't a valid comment.

[zsugabubus]

Thanks for the info, but I used these comments only to make the report look nicer.

[mb720]

I've installed the beta version (1.17.1pre3587-fe39c3e is the correct one, right?) but I still have only a parts of my tridactylrc loaded.

[bovine3dom]

Yeah, that's the right version. Back to the drawing board it is then :)

Thanks for checking.

[mb720]

Thanks for making the effort to fix this 👍

[bovine3dom]

I think the next thing to try is to replace

tridactyl/src/background.ts

Line 89 in fe39c3e

browser.runtime.onStartup.addListener(_ => {

with our own custom event that signals when Tridactyl is ready to accept commands.

[mozbugbox]

#1764 has quite a lot of useful information in it. The trouble is that we are using the browser storage to synchronise configuration between tabs via an onChanged event. config.set says that it is done before the onChanged event is fired, which is a Firefox design choice we have no control over.

I think we need to stop using onChanged -

tridactyl/src/lib/config.ts

Line 1645 in ae40375

browser.storage.onChanged.addListener((changes, areaname) => {

• to synchronise configuration between tabs and create our own solution instead.

I'm not sure I get what the patch really do. As was stated, the onChanged does not guarantee a real change was done. So my understanding is that to make sure that the change actually happened, we just need to check with storage.get periodically untill we get a newvalue from it.

The current onChanged immediately triggered the changeListener without a delay, it basically did what the storage.onChanged did, which is does not guarantee the change to storage was really done.

Anyway, for the issue on tridactylrc with sanitise. If the info in #1764 holds true, it would be that storage.clear returned without finishing it work, then the :source started filling new values to storage. Therefore some added values was deleted on the way.

A straight forward fix will be a setInterval promise which did storage.get(null) from storage until get nothing back, then continue to return from sanitise.

[bovine3dom]

#2181 is my current attempt. It tries to guarantee that storage.set is always set from a globally synchronised configuration object, so that we don't need to care when the storage is actually written to as it will always eventually be right.

[bovine3dom]

Aha, I can reproduce this, sort of. With cpulimit --limit 50 -i npm run run:

sanitise tridactyllocal tridactylsync

set storageloc local

bind g1 tab 1
bind g3 tab 3
bind g5 tab 5
bind g7 tab 7
bind g2 tab 2
bind g4 tab 4
bind g6 tab 6
bind g8 tab 8
bind g9 tab 9

set searchurls.gh  https://github.com/search?utf8=✓&q=

bind J tabprev
bind K tabnext

means that, after a short while, viewconfig --user doesn't show any binds.

However, if I remove either set storageloc local or sanitise tridactyllocal tridactylsync, it works fine. If I replace the sanitise call with either sanitise tridactyllocal or sanitise tridactylsync I still get issues.

It seems likely that the race condition is therefore between set storageloc local and sanitise rather than in any other calls to config.set.

[bovine3dom]

It gets stranger:

sanitise tridactyllocal

set storageloc local

bind g1 tab 1
bind g3 tab 3
bind g5 tab 5
bind g7 tab 7
bind g2 tab 2
bind g4 tab 4
bind g6 tab 6
bind g8 tab 8
bind g9 tab 9

set searchurls.gh  https://github.com/search?utf8=✓&q=

bind J tabprev
bind K tabnext

works on a fresh profile, until I run sanitise tridactyllocal tridactylsync once, and then it never works again.

Even more weirdly, on a fresh profile:

sanitise tridactyllocal tridactylsync

set storageloc local

bind g1 tab 1
bind g3 tab 3
bind g5 tab 5
bind g7 tab 7
bind g2 tab 2
bind g4 tab 4
bind g6 tab 6
bind g8 tab 8
bind g9 tab 9

set searchurls.gh  https://github.com/search?utf8=✓&q=

bind J tabprev
bind K tabnext

1. then leave the computer for as long as you like.
2. Play with the binds, g1, g2 etc. to change the tabs. Run tri.l(browser.storage.local.get()) in the background console.
3. :viewconfig --user, see that it has "worked", run tri.l(browser.storage.local.get()) in the background console, notice that the configuration has disappeared.
4. Go back. The binds no longer work.
5. Run viewconfig --user again. The configuration isn't there.