osmo-gsm-tester[master]: modem: Fix race condition when connect() is called more than...

[Pau Espin Pedrol]

Patch Set 2:

(1 comment)

I guess we agree that removing that sleep() can be done more in the future and doesn't belong to this patch.

https://gerrit.osmocom.org/#/c/3901/1/src/osmo_gsm_tester/ofono_client.py
File src/osmo_gsm_tester/ofono_client.py:

Line 524:         self.cancel_pending_dbus_methods()
> because now we're potentially causing a NoneType exception or forget to res
No, because I'm not recreating the cancellable object every time we connect, as it's not needed (or intended). A cancellable object can also manage several dbus async methods at the same time.

The cancellable is created during construction time and it's expected to last until object cleanup.

And as cleanup() is an API to be used only by suite.py once the test is finished, it also means no method other than destructor is expected to be called after cleanup() is called.

The self.cancellable = None is there to prevent future issues in case we decide for whatever reason to call cleanup() in the destructor, but in any case the "if self.cancellable" and "self.cancellable = None" could be removed. It is basically an optimization in case cleanup() is called more than once.

So, I can leave everything as it is, or I can remove the  "if self.cancellable:" and "self.cancellable=None" lines.
What do you prefer?


-- 
To view, visit https://gerrit.osmocom.org/3901
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic4bb1c6b72c23cd860c33bee7851bca3d0ac0e1b
Gerrit-PatchSet: 2
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-HasComments: Yes