Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

velib_python: fix race condition during service creation #1310

Open
8 of 16 tasks
mpvader opened this issue Jul 14, 2024 · 0 comments
Open
8 of 16 tasks

velib_python: fix race condition during service creation #1310

mpvader opened this issue Jul 14, 2024 · 0 comments
Assignees
@izak @ReinvdZee
Labels
bug medium_prio
Milestone
v3.60

Comments

@mpvader
Copy link
Contributor

mpvader commented Jul 14, 2024 •
edited by izak
Loading

This bug has been in velib_python since at least March 2014, and affects ALL python services that publish data, and uses velib_python.

The bug is that the dbus service name is claimed before the paths and interfaces are registered.

This means that another service scanning this one precisely after the name was claimed but before service construction is complete, may get any number of errors, including:

  • GetItems not implemented
  • DeviceInstance does not exist (causing service to be ignored)

The bug affects faster platforms more. Observed mostly with DSE genset services on Ekrano, but will affect others. Could not be reproduced on the slowest platform, the CCGX, and only with many repeated attempts a Cerbo.

The fix consists of

  • update velib_python/vedbus.py to split name claiming/registration out of the constructor of the VeDbusService object. This allows to first construct the vedbus Object, then set all paths up, and only thereafter register it on the dbus. See this commit as well as the few commits after that.

And then updating the following projects to use the new velib_python and explicitly do service construction after adding all the paths:

  • systemcalc (v3.40)
  • dbus-digitalinputs
  • dbus-modbus-client
  • vrmlogger
  • localsettings (not affected)
  • dbus-generator
  • dbus-modem
  • [dbus-imt-si-rs485tc]
  • dbus-paygo
  • dbus_vebus_to_pvinverter
  • dbus-fzsonick-48tl
  • dbus_pump
  • dbus-tempsensor-relay
  • dbus-parallel-bms
  • dbus-bornay-windplus
@mpvader mpvader added the bug label Jul 14, 2024
@mpvader mpvader added this to the v3.50 milestone Jul 14, 2024
jhofstee added a commit to victronenergy/meta-victronenergy that referenced this issue Jul 17, 2024
@jhofstee
dbus-generator-starter v1.6.14 .. v1.6.16
5f54cec 
v1.6.16: Fix potential race condition with systemcalc
  victronenergy/venus#1310
v1.6.14: Accept invalid dbus-path as non-error condition as well
v1.6.15: Support for DC genset
izak added a commit to victronenergy/dbus-digitalinputs that referenced this issue Sep 19, 2024
@izak
Register dbus service after fully constructing
2fa06eb 
This reduces the possibility of a race condition during scanning.

victronenergy/venus#1310
izak added a commit to victronenergy/dbus_generator that referenced this issue Sep 19, 2024
@izak
update velib_python
c562b13 
This brings it up to date with master and makes it the same as the
other projects. Also makes interface explicit on dbus calls, which
will become important once dbus-generator starts talking to acsystem
services.

victronenergy/venus#1310
izak added a commit to victronenergy/dbus_vebus_to_pvinverter that referenced this issue Sep 19, 2024
@izak
delay service registration to avoid race condition
f030fa5 
Only register the dbus service after all mandatory dbus paths
are populated. Also, update velib_python.

victronenergy/venus#1310
jhofstee added a commit to victronenergy/meta-victronenergy that referenced this issue Sep 19, 2024
@jhofstee
dbus-digitalinputs v0.24
68881e5 
Avoid race conditions when service is scanned before all mandatory
paths are populated.

victronenergy/venus#1310
jhofstee added a commit to victronenergy/meta-victronenergy that referenced this issue Sep 19, 2024
@jhofstee
dbus-generator-starter v1.6.19 & v1.6.20
a084f77 
Bring velib_python up to date
    Avoiding the race condition in system scanning was already fixed
    in Venus 3.40, this just brings the code up to date with the
    other prjects.
    victronenergy/venus#1310
Add Helper relay for connected gensets
    victronenergy/venus#1286
jhofstee added a commit to victronenergy/meta-victronenergy that referenced this issue Sep 19, 2024
@jhofstee
dbus-vebus-to-pvinverter v1.38
5dba5df 
Avoid race conditions when service is scanned before all mandatory
paths are populated.

victronenergy/venus#1310
@mpvader mpvader modified the milestones: v3.50, v3.60 Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@izak izak

@ReinvdZee ReinvdZee

Labels
bug medium_prio
Projects
None yet
Milestone
v3.60
Development

No branches or pull requests
3 participants
@izak @mpvader @ReinvdZee