Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bugfix Issue#95 stale ip address still reported + Fix warnings for li…
…stener events (#147)

* Bug fix jmDNS unable to cache-flush when the cache-flush bit is set

Implementation did not follow closely to the RFC-6762 section 10.2 description and is fixed.

* Fix warnings for listener events

After fixing the cache-flush, ListenerStatus frequently fires new warnings "Service Resolved called for an unresolved event: {}".  This is caused by concurrent modification of the ServiceInfo object.

Signed-off-by: Gary Tse <gary.tse@telekom.de>
  • Loading branch information
garytse authored and kaikreuzer committed Feb 7, 2018
1 parent 94df559 commit be72b17
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 3 deletions.
5 changes: 5 additions & 0 deletions src/main/java/javax/jmdns/impl/DNSRecord.java
Expand Up @@ -1078,4 +1078,9 @@ public void setTTL(int ttl) {
public int getTTL() {
return _ttl;
}

public long getCreated() {
return this._created;
}

}
22 changes: 20 additions & 2 deletions src/main/java/javax/jmdns/impl/JmDNSImpl.java
Expand Up @@ -1346,14 +1346,22 @@ void handleRecord(DNSRecord record, long now) {
final boolean unique = newRecord.isUnique();
final DNSRecord cachedRecord = (DNSRecord) this.getCache().getDNSEntry(newRecord);
logger.debug("{} handle response cached record: {}", this.getName(), cachedRecord);
// RFC 6762, section 10.2 Announcements to Flush Outdated Cache Entries
// https://tools.ietf.org/html/rfc6762#section-10.2
// if (cache-flush a.k.a unique), remove all existing records matching these criterias :--
// 1. same name
// 2. same record type
// 3. same record class
// 4. record is older than 1 second.
if (unique) {
for (DNSEntry entry : this.getCache().getDNSEntryList(newRecord.getKey())) {
if ( newRecord.getRecordType().equals(entry.getRecordType()) &&
newRecord.getRecordClass().equals(entry.getRecordClass()) &&
(entry != cachedRecord)
isOlderThanOneSecond( (DNSRecord)entry, now )
) {
logger.trace("setWillExpireSoon() on: {}", entry);
((DNSRecord) entry).setWillExpireSoon(now);
// this set ttl to 1 second,
((DNSRecord) entry).setWillExpireSoon(now);
}
}
}
Expand Down Expand Up @@ -1425,6 +1433,16 @@ void handleRecord(DNSRecord record, long now) {
}

}

/**
*
* @param dnsRecord
* @param timeToCompare a given times for comparison
* @return true if dnsRecord create time is older than 1 second, relative to the given time; false otherwise
*/
private boolean isOlderThanOneSecond(DNSRecord dnsRecord, long timeToCompare) {
return (dnsRecord.getCreated() < (timeToCompare - DNSConstants.FLUSH_RECORD_OLDER_THAN_1_SECOND*1000));
}

/**
* Handle an incoming response. Cache answers, and pass them on to the appropriate questions.
Expand Down
14 changes: 13 additions & 1 deletion src/main/java/javax/jmdns/impl/ServiceInfoImpl.java
Expand Up @@ -795,6 +795,7 @@ public void updateRecord(final DNSCache dnsCache, final long now, final DNSEntry
// flag for changes
boolean serviceChanged = false;

// When a record is soon to be expired, i.e. ttl=1, consider that as expired too.
if (record.isExpired(now)) {
// remove data
serviceChanged = handleExpiredRecord(record);
Expand All @@ -813,7 +814,18 @@ public void updateRecord(final DNSCache dnsCache, final long now, final DNSEntry
// ServiceEvent event = ((DNSRecord) rec).getServiceEvent(dns);
// event = new ServiceEventImpl(dns, event.getType(), event.getName(), this);
// Failure to resolve services - ID: 3517826
ServiceEvent event = new ServiceEventImpl(dns, this.getType(), this.getName(), this);
//
// There is a timing/ concurrency issue here. The ServiceInfo object is subject to concurrent change.
// e.g. when a device announce a new IP, the old IP has TTL=1.
//
// The listeners runs on different threads concurrently. When they start and read the event,
// the ServiceInfo is already removed/ changed.
//
// The simple solution is to clone the ServiceInfo. Therefore, future changes to ServiceInfo
// will not be seen by the listeners.
//
// Fixes ListenerStatus warning "Service Resolved called for an unresolved event: {}"
ServiceEvent event = new ServiceEventImpl(dns, this.getType(), this.getName(), this.clone());
dns.handleServiceResolved(event);
}
} else {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/javax/jmdns/impl/constants/DNSConstants.java
Expand Up @@ -53,6 +53,7 @@ public final class DNSConstants {
public static final int RECORD_EXPIRY_DELAY = 1; // This is 1s delay used in ttl and therefore in seconds
public static final int KNOWN_ANSWER_TTL = 120;
public static final int ANNOUNCED_RENEWAL_TTL_INTERVAL = DNS_TTL * 500; // 50% of the TTL in milliseconds
public static final int FLUSH_RECORD_OLDER_THAN_1_SECOND = 1; // rfc6762, section 10.2 Flush outdated cache (older than 1 second)

public static final int STALE_REFRESH_INCREMENT = 5;
public static final int STALE_REFRESH_STARTING_PERCENTAGE = 80;
Expand Down

0 comments on commit be72b17

Please sign in to comment.