Ticket #277 (assigned defect)

Opened 8 months ago

Last modified 1 month ago

A single failed "service detection" on the network causes all Devices to remove all of their services

Reported by: kelan Assigned to: dev (accepted)
Priority: critical Milestone:
Component: Core Version:
Keywords: Cc:

Description

In coherence/upnp/core/device.py, Device's service_detection_failed() method does self.remove() (which removes all of the device's services), without first checking if it is actually the device that failed.

This makes it so that a single failure on the network causes all devices to remove their services.

I think it should check if the device argument matches self. I'm not familiar enough with the code to know the best way to compare the two objects. In the attached diff I assumed that comparing their udns was a reasonable approach.

This method was introduced in changeset [487].

Attachments

service_detection_failed_FIX.diff (0.5 kB) - added by kelan on 07.01.2010 02:00:54.
proposed fix

Change History

07.01.2010 02:00:54 changed by kelan

  • attachment service_detection_failed_FIX.diff added.

proposed fix

07.01.2010 10:04:51 changed by dev

  • status changed from new to assigned.

I can't test the correctness of this ticket right now, but from the code we do in

source:/trunk/Coherence/coherence/upnp/core/device.py#L38 

louie.connect( self.service_detection_failed, 'Coherence.UPnP.Service.detection_failed', self)

connect to the signal with sender set to self.
Meaning the method service_detection_failed should only get called when the sender is set to this device.

07.01.2010 19:47:29 changed by kelan

Hmm. Yeah, I wasn't really sure how louie works, but it sounds like the expected behavior you're describing would prevent the issue I was seeing.

However, I did logging in service_detection_failed and saw that self.udn wasn't always equal to devcie.udn. In fact, every one of my devices would get called back with the same (failed) 'device' passed as the argument.

So, maybe the real problem is that louie isn't behaving as expected, in which case my attached diff is more of a "workaround" than a "fix".

03.08.2010 16:31:32 changed by Sunil Mohan

  • priority changed from minor to critical.

First, I confirm this bug. The proposed patch fixes the bug.

This is happening because the louie API wrapper around the dispatcher API does not care about senders and receivers. It is merely dispatching all the signal all the interested parties. We should either have the dispatch mechanism care about senders and receivers or add checks in receivers.

Because this bug ends up destroying most of the functionality of coherence on startup, I am bumping priority.