New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 737224 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ServiceWatcher::DiscoverNewDevices(true) can prevent device discovery callbacks.

Project Member Reported by justincarlson@chromium.org, Jun 27 2017

Issue description

On linux, using mdns, there's a race that can make ServiceWatcher not report devices in some situations when the desired entry is already cached.  The basic scenario is this:

Attempt to list devices under the service name "_ipp._tcp.local" using ServiceDiscoveryDeviceLister (which is mostly a thin wrapper around ServiceWatcher

Start() calls ReadCachedServices(), which immediately resolves the PTR record for _ipp._tcp.local from the cache which contains a printer named 'yourPrinter._ipp._tcp.local'

This bubbles up to ServiceWatcherImpl::OnTransactionResponse, which calls AddService for 'yourPrinter._ipp._tcp.local', which creates a ServiceListener entry in services_, and also schedules a call do DeliverDeferredUpdate with the appropriate record.

So far, so good.

At this point ServiceWatcher::Start() is done and we jump straight into ServiceWatcher::DiscoverNewServices(true).

in ServiceWatcherImpl::DiscoverNewServices, if force_update is true, the first thing we do is clear services_, and then we go on and create the mdns network transactions to look for new services.

At this point DiscoverNewServices() finishes, and we pretty much immediately execute the DeliverDeferredUpdate callback thatt was acheduled earlier.    

However, since services_ was cleared, and the _ipp._tcp.local dns transaction (which is going out to the network) hasn't finished yet, there's no longer an entry in services_ for 'yourPrinter._ipp._tcp.local', which means we never propogate the callback.

Then, when the *network* transactions scheduled by DiscoverNewServices() complete, they find that their contents match what's already in the cache, so the also decline to propagate a callback upwards.

 
Components: -Internals>Network Services>CloudPrint
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2479e9a225a5620ab403734eec1f206dc50fb599

commit 2479e9a225a5620ab403734eec1f206dc50fb599
Author: justincarlson <justincarlson@chromium.org>
Date: Thu Jun 29 20:21:47 2017

Remove the (broken and unneeded) 'force_update' option from ServiceWatcher::DiscoverNewDevices.

This removes the race condition detailed in https://bugs.chromium.org/p/chromium/issues/detail?id=737224.

BUG= 737224 

Review-Url: https://codereview.chromium.org/2963613002
Cr-Commit-Position: refs/heads/master@{#483468}

[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/devtools/device/cast_device_provider.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_impl.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_impl.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_mac.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_mac.mm
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_mdns.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_client_unittest.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_device_lister.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/local_discovery/service_discovery_device_lister.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/media/router/discovery/mdns/dns_sd_device_lister.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/media/router/discovery/mdns/dns_sd_device_lister.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/media/router/discovery/mdns/dns_sd_registry.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/media/router/discovery/mdns/dns_sd_registry_unittest.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_device_lister.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_device_lister_impl.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_device_lister_impl.h
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_device_lister_unittest.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_local_printer_lister.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/browser/ui/webui/local_discovery/local_discovery_ui_handler.cc
[modify] https://crrev.com/2479e9a225a5620ab403734eec1f206dc50fb599/chrome/tools/service_discovery_sniffer/service_discovery_sniffer.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment