Opened 8 years ago

Closed 8 years ago

#1023 closed defect (fixed)

b10-auth crashes with -v for version.bind query

Reported by: jinmei Owned by: vorner
Priority: high Milestone: Sprint-20110628
Component: b10-auth Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: High
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 1.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

(Note: #1022 must be solved before this one)

It can be easily reproduced simply start BIND 10 from source with -v
and do this:

% dig @127.0.0.1 -p <AUTH_PORT> version.bind txt ch

You'll see this:

2011-06-16 17:22:09.877 DEBUG [b10-auth.datasrc] DATASRC_STATIC_FIND, looking for 'version.bind./TXT'
Assertion failed: (px != 0), function operator->, file /opt/local/include/boost/smart_ptr/shared_ptr.hpp, line 418.

It doesn't happen without -v. I suspect it's related to DEBUG level
logging, but don't have time to look into it right now.

I believe this should be fixed before the next release, so I'm pushing
this ticket to the current sprint.

Subtickets

Change History (6)

comment:1 Changed 8 years ago by jinmei

Additional info: the reason for this seems that node->getRRset() is NULL
(shared pointer) in this context:

inline void
HotCacheImpl::insert(const CacheNodePtr node) {
    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_CACHE_INSERT).
        arg(node->getRRset()->getName());

This can be confirmed more explicitly by adding an assert():

    assert(node->getRRset());
    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_CACHE_INSERT).
        arg(node->getRRset()->getName());

Fixing this is probably is easy, but I suspect we should learn a bit
more than the immediate lesson from this incident:

  • We should be more careful about logging argument so that it wouldn't cause server crash. Even in the worst case we should catch unexpected case more gracefully by throwing an exception.
  • We should use highest possible verbosity for logging in our unit tests.

I'll create separate tickets for these two.

comment:2 Changed 8 years ago by stephen

  • Owner set to stephen
  • Status changed from new to assigned

comment:3 Changed 8 years ago by stephen

As this is a small change and is dependent on #1022, the fix was made in branch trac1022 at the same time that #1022 was addressed.

comment:4 Changed 8 years ago by stephen

  • Owner changed from stephen to UnAssigned
  • Status changed from assigned to reviewing

comment:5 Changed 8 years ago by vorner

  • Owner changed from UnAssigned to vorner

Taking this as well, as it is in the same branch as #1022.

comment:6 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 1
  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged and pushed to master, bf41f8dc2265ca0cd9ffb8b8c11047291e69ca3c

Note: See TracTickets for help on using tickets.