backend: plugins: Race condition with plugins on some systems

[illume]

There's a race condition for when files are copied into place when doing an npm start from within a plugin, and the backend being able to see if a file exists inside GeneratePluginPaths.

This is the error in the backend:

{
  "level":"info",
  "pluginPath":"/home/user/.config/Headlamp/plugins/change-logo/main.js",
  "source":"/home/runner/work/headlamp/headlamp/backend/pkg/plugins/plugins.go","line":131,
  "error":"stat /home/user/.config/Headlamp/plugins/change-logo/main.js: no such file or directory",
  "time":"2024-03-06T17:42:59+05:30",
  "message":"Not including plugin path, main.js not found"
}

But when doing an ls on the file it is there. When npm start inside the plugin is stopped, the issue disapears.

So there seems to be a race condition.

To reproduce

# Have backend running in one terminal.
make run-backend

# In the other terminal

cd examples/plugins/change-logo/
npm i
npm start

Notes

• plugins.go@HandlePluginEvents is the code which watches for events and runs the path generation
• headlamp-plugin.js@copyToPluginsFolder is what copies the plugin files using some webpack code "FileManagerPlugin". Not sure if it does atomic moves, or if removes and deletes inside.

[HariCharanK]

I'd like to work on this. You can assign it to me.

[HariCharanK]

A naive solution could be sleep-and-retry ( we can have a config for sleeptime and max_no_of_retries )

The issue with the above approach is that even after dist/main.js and package.json are copied, there could be files inside dist/ that are still in progress.

But upon looking at the code in headlamp-plugin.js (ss below), I found out that FileManagerPlugin performs actions sequentially, so package.json is copied only after the entire dist/ dir is copied. I did some research, and it does not leverage any multi-threading.

\

We can do sleep-and-retry for package.json , if it is found, main.js is expected to be found on the first try (unless there is some other error). Config values tbd on a worst-case basis.

This is not perfect. There could be a case where the dist/ dir is larger than expected, and the worst-case sleeptime value we assumed is not sufficient.

Also, if we set a high sleeptime, assuming the worst case, we will unnecessarily be stalling for lightweight plugins. To avoid this, we can have incremental sleep time. Instead of 10s of sleep for 3 retries, we can have 2, 10, 20s of sleep before each try.

[HariCharanK]

The above asynchronous polling style approach, tho it is easy to implement, has some drawbacks.

@illume , any thoughts on this ?

Another solution could be sending a synchronous signal from headlamp-plugin.js to plugins.go after copying files. We can send an ACK/NACK packet on a particular port and receive it on the other end. This is somewhat complex.

[illume]

Thanks for looking into this @HariCharan-001

I wonder did you manage to reproduce the issue yourself?

It would be a pity if we have to introduce a sleep all the time if we don't need to. This delays the page reload a bit. But this could be a solution if nothing else can be found.

I was wondering about was that we always copy a certain file last. For example the package.json file. Then watch on this package.json file in the backend. Since we know it will always be last, this is the one to watch.

Additionally, the files could be first produced in a tmp folder, and then moved atomically. This would stop one race condition from happening. Instead of the backend watch noticing a file and write then it would just see moves, which should happen atomically (and instantly according to the backend file watcher).

As an aside, I saw this page on atomic moves of folders. https://stackoverflow.com/questions/307437/moving-a-directory-atomically I'm not sure how portable things are on Mac / Windows though.

[HariCharanK]

Yes, I was able to reproduce the issue.

"I was wondering about was that we always copy a certain file last. For example the package.json file" Right, right, this is what I was talking about.

I'll take a look at atomic moves.

[illume]

No one got to this one yet. Moving to the next release milestone.