Bug #419

Deleting a channel from a model does not seem to delete the channel

Added by Espen Solbu over 5 years ago. Updated over 5 years ago.

Status:ClosedStart date:04/30/2012
Priority:NormalDue date:
Assignee:Android Dashboard Developers% Done:

0%

Category:-Spent time:-
Target version:2.0 Multiple models

Description

After a channel has been deleted from a Model, It will still report in the logger that it is receiving updates or resets.
(Unregister not working?)

Something is still holding a reference to it.
  • perhaps listeners not properly unregistered?
  • Stored in some treemap still? (dashboard?)

20120501_log-channel-reset.txt Magnifier (10.6 KB) Hans Cappelle, 05/01/2012 10:11 am


Related issues

Related to Bug #421: Channels from multiple models listen to channel updates Closed 05/01/2012
Related to Bug #420: AD1 raw and AD2 raw registers twice Closed 05/01/2012
Related to Bug #348: Performance issues Closed 02/10/2012
Related to Bug #424: Deleting or adding a channel, sets source channel to "Non... Closed 05/03/2012

Associated revisions

Revision 387
Added by Espen Solbu over 5 years ago

refs #348,#419,#420,#420 Debugging performance issues, added logging, removed passing parselled channel back unless neccesary, added more attempts at unregister, added finish() to ActivityChannelConfig to try to prevent it from holding references.

Revision 392
Added by Hans Cappelle over 5 years ago

refs #419 unregister fails for resetting broadcast listener, also too many resets, check channel collections

Revision 393
Added by Hans Cappelle over 5 years ago

refs #419 logging channel object ID for debug purpose

Revision 395
Added by Espen Solbu over 5 years ago

refs #419 Moved register code out of ctor. Seem to have fixed the delete problem

Revision 396
Added by Espen Solbu over 5 years ago

refs #419 Needed to add a register when adding a new channel. This seems to work now...

History

#1 Updated by Espen Solbu over 5 years ago

The channels do get properly deleted from the database, however still persists across application restarts.

#3 Updated by Hans Cappelle over 5 years ago

Are you sure about this? I debugged removing a channel and it seems to be properly removed no more receiving broadcasts. You can put a breakpoint at

Channel$1 [line: 617] - onReceive(Context, Intent)

and then check the channel ID (be careful not to look at the inner class object id of the listener, you have to open that first this reference (the one with the dollar 1 in the name) and then check the this inside there. That ID matches the ID of the channel object itself.

I checked and there I can add/delete channels using the edit model option. Also if I restart the app this is respected.

I'm looking for the log statements created to check this if channel is closed and I get one log but that's it. Probably because once you set the closed member to true there are still some channels in buffer to be handled or something like that. Not a big issue I would say since it's only a single log occurence.

#4 Updated by Hans Cappelle over 5 years ago

The resetting however is another story. For that receiver the channel that was removed still got an event.

I added a log file extract showing a situation where I started with 3 channels, full pack, single cell and description3. There I removed description3 and got a single error message about it still receiving events. Nothing more (not sure I tested long enough in this logged case but I did in other cases for sure). So for me the channel value broadcast seems to be removed properly.

However I quit the simulator there generating the resetting events. These are still received for the description3 channel. Will have to look into detail some other time.

#5 Updated by Espen Solbu over 5 years ago

I think there is a problem yes. I don't think any of the listeners properly unregister. I have not deeply investigated if the error message keeps coming while running the simulator.

The fact that the RESET broadcast is still captured by the deleted channel (even after restart of app) is bothering me alot though. In my understanding this means 2 things:
1. The channel still "exists" somewhere, even though it has been deleted
2. The channel still listens for the broadcasts, even though it supposedly unregistered
3. I assume this also applies to the update broadcast, though i have not investigated this

I suspect that the channel still lives in various activities (source channel, channel list in dashboard etc). I suspect that this causes problems destroying the channel. It still does not explain why it will not unregister.

i was planning to use the packagemanager to list out all listeners and see if can iterate then and unregister them...
Or try to have the channel unregister itself if it has been closed...

I am also suspecting that it can have something to do with loading the objects from database.

In general, i am starting to become VERY sceptical of using broadcasts between objects not based on Activity class. (I dislike referencing the context within these classes)

If we can't sort this, i have the following possible workarounds
  • Redo database access, load all channels from database into list on server from database. Use this list to "disable"(unregister receivers) channels not attached to current model, and to enable(register receivers) channels attached to current model. Should also make it easier to detect duplicate objects for same channel.
  • Stop using broadcasts for chaining channels. Use interface instead
  • Go ahead with making channel updates cyclic instead of event driven
    • "Derived Channels" should poll source channels. (source channel should exist upon creation of derived channels)

#6 Updated by Hans Cappelle over 5 years ago

  • Status changed from New to In Progress

Espen

I had a quick view to the channel issues. I added their object IDs (java object in memory) to the logging (see latest revision). And did some tests to see what is happening. No solution yet but some issues already to be fixed. Just need the time to fix them.

Some conclusions:

*) I believe problem is only for command broadcasts (reset), not for channel value updates.

*) The way the channels are loaded from the database is one issue. There the channel ctor is used so a new object is created and it is registered for the commands.

*) On create of the channel activity, the one where we can edit the channel properties, a channel is also created, so registered. If you then decide to not save that channel it will still be registered. Also it looks like all channels are fetched again from db on opening that activity resulting in the current amount of channels + 1 being registered (as new objects).

The test case I did:

start app with clean model (no custom channels), only fixed channels registered = OK

05-03 11:46:42.495: D/Channel(506): None: Resetting self
05-03 11:46:42.516: D/Channel(506): Channel None registered. Channel Object ID: 1157008896
05-03 11:46:42.516: D/Channel(506): AD1: Resetting self
05-03 11:46:42.525: D/Channel(506): Channel AD1 registered. Channel Object ID: 1157013616
05-03 11:46:42.535: D/Channel(506): AD2: Resetting self
05-03 11:46:42.545: D/Channel(506): Channel AD2 registered. Channel Object ID: 1157015288
05-03 11:46:42.585: D/Channel(506): RSSIrx: Resetting self
05-03 11:46:42.595: D/Channel(506): Channel RSSIrx registered. Channel Object ID: 1157016960
05-03 11:46:42.595: D/Channel(506): RSSItx: Resetting self
05-03 11:46:42.615: D/Channel(506): Channel RSSItx registered. Channel Object ID: 1157018760

not unregistered at close, probably not needed? = TBD

On create of channel crud activty already channel registered. The channel you want to create and any other channel that was already on the model. If user returns without saving (no channel created) still all channels registered, even the one that never was saved! = NOK

I didn't the check the object IDs but I believe new channels are created on database load so these ID's would also be different then so all the new registered channels from DB are duplicates = NOK

(btw don't mind the object IDs in this log since I started and closed the app and so no consistency here, need a single session to verify)

05-03 11:49:58.605: D/Channel(506): description: Resetting self
05-03 11:49:58.615: D/Channel(506): Channel description registered. Channel Object ID: 1157293632
05-03 11:49:58.785: D/Channel(506): description: Resetting self
05-03 11:49:58.795: D/Channel(506): Channel description registered. Channel Object ID: 1157004800
05-03 11:49:59.415: I/ChannelConfig(506): Start the server service if it is not already started
05-03 11:49:59.425: I/ChannelConfig(506): Try to bind to the service
05-03 11:49:59.435: D/ChannelConfig(506): Channel Id is: -1
05-03 11:49:59.975: I/ChannelConfig(506): Bound to Service
05-03 11:49:59.975: I/ChannelConfig(506): Fetch channel -1 from Server
05-03 11:49:59.985: D/ChannelConfig(506): Channel config launched with attached channel: Description2
05-03 11:49:59.985: D/ChannelConfig(506): channel context is: android.app.Application@44f40150
05-03 11:49:59.995: D/ChannelConfig(506): channel context is: android.app.Application@44f40150
05-03 11:49:59.995: D/ChannelConfig(506): Configure existing Model object (id:2)

on actual save of a channel still parcelled objects used, check!? Not sure if this still causes issues. => TBD

05-03 11:57:06.695: D/ChannelConfig(506): Sending Parcelled channel back: Description:Description2, silent: false

on delete of channel unregister fails with exception. I've seen 2 reasons for these kind of exceptions. On no longer being in the right context. The broadcastreceiver only lives for the broadcast itself so in the onReceive method. But that doesn't seem to be the issue here. Other cause I've seen for this is a non existing object. This could be the case if (theory here, not verified) we always reload the channels for a model from db so new objects are created and at this point we try to unregister objects with the wrong id. => TODO

05-03 12:01:41.145: I/Channel(506): Try to close Channel Description1
05-03 12:01:41.145: D/Channel(506): Description1: Removing broadcast listener
05-03 12:01:41.145: D/Channel(506): Description1: Removed Listener Success
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): unregister reset broadcast receiver on channel failed
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): java.lang.IllegalArgumentException: Receiver not registered: biz.onomato.frskydash.domain.Channel$2@44fddf00
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.app.ActivityThread$PackageInfo.forgetReceiverDispatcher(ActivityThread.java:793)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.app.ContextImpl.unregisterReceiver(ContextImpl.java:814)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.content.ContextWrapper.unregisterReceiver(ContextWrapper.java:331)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at biz.onomato.frskydash.domain.Channel.close(Channel.java:564)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at biz.onomato.frskydash.domain.Model.removeChannel(Model.java:254)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at biz.onomato.frskydash.activities.ActivityModelConfig$4.onClick(ActivityModelConfig.java:356)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at com.android.internal.app.AlertController$ButtonHandler.handleMessage(AlertController.java:158)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.os.Handler.dispatchMessage(Handler.java:99)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.os.Looper.loop(Looper.java:123)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at android.app.ActivityThread.main(ActivityThread.java:4627)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at java.lang.reflect.Method.invokeNative(Native Method)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at java.lang.reflect.Method.invoke(Method.java:521)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:868)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:626)
05-03 12:01:41.205: E/class biz.onomato.frskydash.domain.Channel(506): at dalvik.system.NativeStart.main(Native Method)

Finally on close there were no channels on the model, on relaunch of app indeed only fixed channels registered

#7 Updated by Hans Cappelle over 5 years ago

1) For the registering of command broadcasts issue I believe we should take the register out of the ctor. That would be a first step.

2) Second step would be to see where we really need to create new channels/models and where not. Now on saving a model for instance all new objects are created. Not needed. Only on startup or saving new objects are needed. Otherwise we can get them from the available collection.

details

  • On create of dashboard activity load all models and their channels WITHOUT registering. Here we create new objects from database entries and store them in collections (program memory). For current Model we can already store on FrSkyServer (service) object, for other models I have to see where they are/can be stored. For all next steps we have to work on these objects! (so we need to foresee public accessors).
  • Also on create of dashboard we can load all the fixed channels and register them (with separate method per channel). This can be done since these objects will be active no matter what model is selected. The registering and unregistering of channels for the command broadcast on startup need to be limited to the fixed channels (+ hub if activated)
  • The channels that are on the currently selected model can be registered and unregistered dynamically. On switching models we need to unregister the channels from previous and register the ones from the new current model.
  • For all operations we work on the Model and Channel objects in the collections. This way we always have the same and only instance to register/unregister or do whatever action is needed.
  • On saving a model or channel we create a new channel instance, save in db, register for broadcasts and keep working on that one.

#8 Updated by Espen Solbu over 5 years ago

I agree to most of it. (same as my "first" workaround above)

Hans Cappelle wrote:

1) For the registering of command broadcasts issue I believe we should take the register out of the ctor. That would be a first step.

OK

2) Second step would be to see where we really need to create new channels/models and where not. Now on saving a model for instance all new objects are created. Not needed. Only on startup or saving new objects are needed. Otherwise we can get them from the available collection.

OK

details

  • On create of dashboard activity load all models and their channels WITHOUT registering. Here we create new objects from database entries and store them in collections (program memory). For current Model we can already store on FrSkyServer (service) object, for other models I have to see where they are/can be stored. For all next steps we have to work on these objects! (so we need to foresee public accessors).

Almost, but i want the collection for all channels stored in the FrSkyServer, and populated when server starts (not the dashboard). I think we need to have one collection for all channels, and copy references from this.
Note. Channels with id < 0 are fixed channels, and not stored in database.
It should be easy to iterate collection on change model (first disable All derived channels, then enable only the ones that belongs to the new current model. Source channels should never be registered for channelupdate broadcasts anyways.

  • Also on create of dashboard we can load all the fixed channels and register them (with separate method per channel). This can be done since these objects will be active no matter what model is selected. The registering and unregistering of channels for the command broadcast on startup need to be limited to the fixed channels (+ hub if activated)

See above comment

  • The channels that are on the currently selected model can be registered and unregistered dynamically. On switching models we need to unregister the channels from previous and register the ones from the new current model.

See above comment

  • For all operations we work on the Model and Channel objects in the collections. This way we always have the same and only instance to register/unregister or do whatever action is needed.

Yes

  • On saving a model or channel we create a new channel instance, save in db, register for broadcasts and keep working on that one.

Not sure i understand this one. I would think it would be better to save the existing instance instead of making a new one to prevent this duplication of channels...
Alternatively, only save to database when we exit application. Only work with instances in memory otherwise

#9 Updated by Espen Solbu over 5 years ago

I think this works properly now

#10 Updated by Espen Solbu over 5 years ago

  • Status changed from In Progress to Closed

#11 Updated by Hans Cappelle over 5 years ago

details

  • On create of dashboard activity load all models and their channels WITHOUT registering. Here we create new objects from database entries and store them in collections (program memory). For current Model we can already store on FrSkyServer (service) object, for other models I have to see where they are/can be stored. For all next steps we have to work on these objects! (so we need to foresee public accessors).

Almost, but i want the collection for all channels stored in the FrSkyServer, and populated when server starts (not the dashboard). I think we need to have one collection for all channels, and copy references from this.
Note. Channels with id < 0 are fixed channels, and not stored in database.
It should be easy to iterate collection on change model (first disable All derived channels, then enable only the ones that belongs to the new current model. Source channels should never be registered for channelupdate broadcasts anyways.

Yes server indeed.

Wow is the -1 id for non persistent channels. I might have mistaken that in refactoring the DB code. I thought it was for non yet saved objects!? However I think I respected existing code functionality while refactoring so might not be an issue.

OK for source channels.

  • On saving a model or channel we create a new channel instance, save in db, register for broadcasts and keep working on that one.

Not sure i understand this one. I would think it would be better to save the existing instance instead of making a new one to prevent this duplication of channels...
Alternatively, only save to database when we exit application. Only work with instances in memory otherwise

No I ment that if we open the crud activity we have a new channel object but not in storage yet. We work on that object and only if the user selects save it can be persisted and added to the collection on server level. Like your last point indeed.

#12 Updated by Espen Solbu over 5 years ago

Hans Cappelle wrote:

details

  • On create of dashboard activity load all models and their channels WITHOUT registering. Here we create new objects from database entries and store them in collections (program memory). For current Model we can already store on FrSkyServer (service) object, for other models I have to see where they are/can be stored. For all next steps we have to work on these objects! (so we need to foresee public accessors).

Almost, but i want the collection for all channels stored in the FrSkyServer, and populated when server starts (not the dashboard). I think we need to have one collection for all channels, and copy references from this.
Note. Channels with id < 0 are fixed channels, and not stored in database.
It should be easy to iterate collection on change model (first disable All derived channels, then enable only the ones that belongs to the new current model. Source channels should never be registered for channelupdate broadcasts anyways.

Yes server indeed.

Wow is the -1 id for non persistent channels. I might have mistaken that in refactoring the DB code. I thought it was for non yet saved objects!? However I think I respected existing code functionality while refactoring so might not be an issue.

Err. -1 is "new/undefined". All other negative numbers are defined/fixed channels created in code (sourcechannels)

OK for source channels.

  • On saving a model or channel we create a new channel instance, save in db, register for broadcasts and keep working on that one.

Not sure i understand this one. I would think it would be better to save the existing instance instead of making a new one to prevent this duplication of channels...
Alternatively, only save to database when we exit application. Only work with instances in memory otherwise

No I ment that if we open the crud activity we have a new channel object but not in storage yet. We work on that object and only if the user selects save it can be persisted and added to the collection on server level. Like your last point indeed.

Sure.
I have a mix of these actually :(
Model Config, will autosave when you leave the activity, while channel Config requires you to save.
If we move all DB access to server start and stop, we can more easily make this behavior the same for all activities.

Also available in: Atom PDF