Ticket #5618 (new defect)

Opened 7 years ago

Last modified 6 years ago

PS should drop handles causing InspectHandles failing

Reported by: chihyu Owned by: gdesmott
Priority: normal Milestone: 9.1.0-cancelled
Component: presence-service Version:
Keywords: 8.2.0:? joyride-2288:+ Cc: joe, mbletsas, Charlie, morgs
Action Needed: design Verified: no
Deployments affected: Blocked By:
Blocking:

Description

Tested with three different machines: build 653, joyride 1454 (clean install), joyride 1454 (olpc-updated from build 653).

All machines were connected to mesh network 1 (school server).

Go to Neighborhood view, hover mouse over buddy icons -- some buddy does not have a name, but can possibly be added as a friend or invited to an activity.

Attachments

653_nameless.png (52.4 kB) - added by chihyu 7 years ago.
1454_1_nameless.png (51.9 kB) - added by chihyu 7 years ago.
1454_2_nameless.png (51.9 kB) - added by chihyu 7 years ago.
dec20-1.log (24.2 kB) - added by chihyu 7 years ago.
presenceservice.log (Dec. 20)
dec21-1.log (7.2 kB) - added by chihyu 7 years ago.
presenceservice.log (Dec. 21) 1 of 2
dec21-2.log (7.2 kB) - added by chihyu 7 years ago.
presenceservice.log (Dec. 21) 2 of 2

Change History

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

Changed 7 years ago by chihyu

  Changed 7 years ago by jg

  • owner changed from morgs to marco
  • component changed from connect-activity to sugar
  • milestone changed from Never Assigned to Future Release

  Changed 7 years ago by marco

  • keywords update.1? added
  • milestone changed from Future Release to Retriage, Please!

Probably a presence service bug. Seems like it would be worth to spend some time to at least track down the issue for Update.1.

  Changed 7 years ago by jg

  • keywords update.1? removed
  • priority changed from normal to high
  • milestone changed from Retriage, Please! to Update.1

Whether we can fix it will of course, depend on the fix...

  Changed 7 years ago by marco

  • owner changed from marco to morgs
  • component changed from sugar to presence-service

follow-up: ↓ 6   Changed 7 years ago by morgs

chihyu, any chance of logs? http://wiki.laptop.org/go/AttachingSugarLogsToTickets

I can't reproduce this myself and need the presenceservice.log of a laptop while it is seeing a buddy with no name.

in reply to: ↑ 5   Changed 7 years ago by chihyu

I am attaching three possible presenceservice.log files during the time period I created this ticket. Thanks a lot!

Replying to morgs:

chihyu, any chance of logs? http://wiki.laptop.org/go/AttachingSugarLogsToTickets I can't reproduce this myself and need the presenceservice.log of a laptop while it is seeing a buddy with no name.

Changed 7 years ago by chihyu

presenceservice.log (Dec. 20)

Changed 7 years ago by chihyu

presenceservice.log (Dec. 21) 1 of 2

Changed 7 years ago by chihyu

presenceservice.log (Dec. 21) 2 of 2

  Changed 7 years ago by morgs

  • cc Collabora added

In the logs, attempts to get aliases, buddy properties, current activity and activities for a handle.

It looks like the handle disappeared by the time we want to get these, in fact probably before we create a Buddy object and connect to the disappeared signal.

  Changed 6 years ago by marco

  • milestone changed from Update.1 to Future Release
<morgs> marcopg: #5618 seems to be when a buddy appears and disappears before we can create the buddy object and hook the disappeared signal, so we have a phantom buddy which has no telepathy handle.
<morgs> I haven't heard of it happening outside 1cc.
<marcopg> morgs: I tend to think it's not critical for Update.1 then and should be punted
<morgs> marcopg: I agree, changes would be too invasive to fix a really small issue

  Changed 6 years ago by morgs

  • cc Collabora removed
  • owner changed from morgs to Collabora

  Changed 6 years ago by marco

  • keywords 8.2.0:? added
  • milestone changed from Future Release to 8.2.0 (was Update.2)

If we can't do anything without more info let's close this one.

  Changed 6 years ago by mstone

  • cc joe, mbletsas, Charlie added
  • next_action set to communicate

Dear Collabora - we need to know whether this is still a problem. Could you please comment?

  Changed 6 years ago by gdesmott

According logs, I observed the following failures:

InspectHandles failed in telepathy_plugin.py:_contacts_online

Error when fetching alias, buddy properties, activities, current activity in presenceservice.py: _contacts_online

BuddyInfo.GetProperties failed in linklocal_plugin.py: identify_contacts

As the first failure is in telepathy_plugin.py:_contacts_online, we could probably drop the handle if InspectHandles fails. Problem is, I don't think we can identify which handle cause the failing if more than one handle is passed to InspectHandles.

  Changed 6 years ago by gdesmott

According smcv, the futur Inspectotron should solve this problem.

  Changed 6 years ago by mstone

  • priority changed from high to normal

  Changed 6 years ago by gdesmott

  • owner changed from Collabora to gdesmott

After further discussions we agreed to write a fallback path instead of waiting for the futur API. Will fix it.

  Changed 6 years ago by mstone

The detailed problem is that the PS uses a batch query API (InspectHandles) that has no way to indicate that individual sub-queries failed. Inspectotron is a newer batch-query API that does not suffer from this problem; however, it is not scheduled for use in 8.2.0. In the mean-time, Sjoerd suggests a simple PS workaround: requery results one at a time when the overall batch query fails.

  Changed 6 years ago by gdesmott

  • next_action changed from communicate to review

  Changed 6 years ago by morgs

  • cc morgs added

  Changed 6 years ago by gdesmott

Merged to master, thanks to Morgs and Sjoerd for the review.

If you want this fix in 8.2.0 I can merge it to the stable branch (sucrose-0.82) too and do a 0.82.1 stable release.

  Changed 6 years ago by mstone

  • next_action changed from review to package

When you have tested that the patch fixes the issue, please see that this makes its way into joyride.

  Changed 6 years ago by gdesmott

Merged to the sucrose-0.82 stable branch.

  Changed 6 years ago by gdesmott

  • summary changed from machine with no name to PS should drop handles causing InspectHandles failing

  Changed 6 years ago by gdesmott

  • next_action changed from package to add to build

I released and packaged PS 0.82.1 which contains this fix. Package has to be tagged to reach Joyride.

  Changed 6 years ago by gdesmott

  • next_action changed from add to build to test in build

sugar-presence-service 0.82.2 is now in Joyride so this bug should be fixed. Will do some tests in build to be sure it didn't introduced regression but the bug is very rare and hard to reproduce.

  Changed 6 years ago by gdesmott

  • next_action changed from test in build to qa signoff

Did some tests and things seemed to work fine.

  Changed 6 years ago by mstone

Please tag the ticket with your test results according to http://wiki.laptop.org/go/Trac_conventions

  Changed 6 years ago by gdesmott

  • keywords joyride-2288:+ added

  Changed 6 years ago by daf

Ignoring InspectHandles errors doesn't seem like it addresses the root problem: the problem is that the presence service has invalid handles. Either the connection manager is not telling the presence service when a handle is invalidated, or it is generating that information and the presence service is not paying attention to it.

It's hard to tackle this without being able to reproduce it, though.

  Changed 6 years ago by gdesmott

  • next_action changed from qa signoff to design
  • milestone changed from 8.2.0 (was Update.2) to 9.1.0

A possible scenario could be:

- Buddy is online. PresenceChanged(online) is fire from the CM
- Right after, buddy becomes offline, PresenceChanged(offline) is fired
- PS is scheduled and catch the first signal: the handle is not valid anymore

Move this bug to 9.1 as we workarounded it for 8.2 and we can't really debug it without a test framework.

Note: See TracTickets for help on using tickets.