Mark,
Am 18.11.19 um 02:37 schrieb Mark
Webb-Johnson:
I don’t think this fix is correct. Seeking clarification of the
thinking behind it.
Currently, MyOvmsServerV2Reader is initialised to 0.
Then, OvmsServerV2::OvmsServerV2 creates the reader (and
sets MyOvmsServerV2Reader to it) if it is zero.
Then OvmsServerV2::~OvmsServerV2 deregisters (clears) the
reader, but does not set MyOvmsServerV2Reader to 0.
The change is to OvmsServerV2::OvmsServerV2 to
register the reader (without setting MyOvmsServerV2Reader)
if MyOvmsServerV2Reader is 0.
This seems to leak one reader every time the
OvmsServerV2 is stopped/started, as without storing
in MyOvmsServerV2Reader, the old reader will never be cleared
upon deregistering. Or am I missing something?
You miss the MyOvmsServerV2Reader being given to the second
registration call to be reused.
The current implementation of reader registration is allocating one
of 32 available bits for a new reader. The bit allocation is
permanent, the servers accordingly only allocate their ids once.
But the reader entry in the list of active notification readers
isn't permanent. To be able to re-register for a previously
allocated bit number, I introduced the second RegisterReader()
method some time ago. That was for the webserver context --
registering a new id on every new websocket connection would run out
of ids very soon.
It became apparent (from a user report on the weekend) that both v2
& v3 servers did not register as readers on a second start, so
have the same issue.
You can test the previous implementation and the fix by watching
"notify status".
It seems that the better solution is to
change OvmsServerV2::~OvmsServerV2 to set MyOvmsServerV2Reader=0
after deregistering (which is the original bug, imho)?
That would actually leak reader bits due to the permanent
allocation.
It would have been an option to add a bit mask for the use/free
status so you don't need to permanently store and reuse the
allocated reader id. IIRC I decided against that because the
v2&v3 servers had that scheme of permanent storage. Thinking
about it now, that would be a more clean, more standard way option
for the notify API, maybe we should do that.
Regards,
Michael
Regards, Mark.
Begin forwarded message:
Subject: [openvehicles/Open-Vehicle-Monitoring-System-3]
f84837: Server V2 & V3: fix registration for
notifications...
Date: 17 November 2019 at
11:22:46 PM HKT
_______________________________________________
OvmsDev mailing list
OvmsDev@lists.openvehicles.com
http://lists.openvehicles.com/mailman/listinfo/ovmsdev
--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26