ccoverstreet / Jablko

Smart Home Interface powered by Go.
MIT License
1 stars 0 forks source link

Error when apply changes is accidentally pressed twice #88

Closed ccoverstreet closed 3 years ago

ccoverstreet commented 3 years ago

Error when apply is pressed twice in the admin panel. Haven't looked into this too closely yet.

Screenshot_20210529_181127

ccoverstreet commented 3 years ago

It seems like the issue is a kill command being sent to the same process twice. Need to add some safeguard against this. It doesn't seem to a critical issue since everything still runs fine.

ccoverstreet commented 3 years ago

One solution may be to add an IsStopped flag on each subproc and checking if that flag has been set already

ccoverstreet commented 3 years ago

This is a more general behavior of golang Cmd and Process. When a Cmd is first created, the Process field is nil until the Run or Start method is called. This results in a nil-pointer dereference on a double Process.Kill call, which causes the panic.

This problem can be fixed by also checking the status of the command.Process field. If the Process field is nil, StopJMOD should return an error and not panic.

Sidenote: the restart logic used in core/admin/admin.go should be moved into a core/modmanager function as this behavior should not vary across Jablko (although I'm not sure if I will have many callers using this function).

ccoverstreet commented 3 years ago

Three retries over three seconds has been implemented. A flag should be added to indicate whether the process is in the act of exiting to prevent retries on clearly invalid attempts.