[ntp:bugs] [Bug 596] ntpd dies after 2 days

[bugzilla at ntp.isc.org]

http://bugs.ntp.isc.org/show_bug.cgi?id=596



----------------------------------------------------------------------------
Additional Comments From mayer at ntp.org (Danny Mayer)
Submitted on 2006-04-16 04:33
> Yes, and get_full_recv_buffer() should only be called if there is a full
> buffer to fetch.

You need to leave it to get_full_recv_buffer() to decide whether or not anything
is available.

> If we're gonna change this, the new code should be at least as fast as
> the old code.

Do you have reason to believe it isn't?

>> I just added a check to log an error if the full list is empty but
>> full_recvbuffs is non-zero.
> 
> What about the case where full_recvbuffs is zero but the list is not
> empty?

Then you'll get a message in the log with a negative value when the list does
become empty. I just added this to my code, it's not available anywhere yet.

>> The change to ntpd.c is essential as I noticed that on Windows the
>> entries in the list were almost never emptied.
> 
> Doesn't that implies that full_recvbuffs was zero when the list was not?

Yes, but I can't imagine why that would be since they must change in synch and
the locking guarantees that at least on Windows.

>> Please remember that Windows is multithreaded and needs to be handled
>> properly.
> 
> Of course.  And how is the Windows multithread case different from SIGIO?

They're not related. The Windows function library is threadsafe and malloc() and
free() on Unix breaks when using SIGIO (at least glibc). See bug #572 for an
example of this. This is also the problem that Brian saw in the coredump that
Paul sent him.

>>  Is there still a problem with the counter value? Probably,
>> but it belongs in the recvbuf code and not in ntpdmain.
> 
> "it"?

Testing for errors in full_recvbufs belong in the recvbuf code.

> Are you telling me that it is thread-safe to add/remove an entry from a
> list (and deal with the node creation and initialization/destruction)
> but it is not thread-safe to increment/decrement a (volatile) counter in
> that same code path?

There does appear to be an issue with full_recvbufs but since I made the change
I haven't seen any issues or errors being logged when the full list has become
empty. Like I said this belongs in recvbuf.

Danny

-- 
Danny Mayer <mayer at ntp.org>



------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.