Opened 12 years ago

Closed 12 years ago

#2749 closed Defect (fixed)

Source code needs to be compiled with -D_REENTRANT

Reported by: wereHamster Owned by: charles
Priority: Normal Milestone: 1.80
Component: Transmission Version: 1.70
Severity: Normal Keywords:
Cc: jch@…

Description

Is there a bug somewhere or can that error be silenced without any side effects? FYI, errno 11 is Resource temporarily unavailable

Attachments (3)

tr-dht.diff (1.0 KB) - added by charles 12 years ago.
maybe we could log the first error, but not log subsequent identical errors... jch, werehamster, how does this look to you?
0001-Fix-flood-of-dht_periodic-events.patch (1.6 KB) - added by wereHamster 12 years ago.
0001-third-party-dht-needs-to-be-compiled-with-PTHREAD_CF.patch (2.0 KB) - added by wereHamster 12 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 12 years ago by charles

  • Cc jch@… added

comment:2 Changed 12 years ago by charles

wereHamster: it looks like we could silence this DHT error without any side-effects. cc'ing jch for a second opinion...

Changed 12 years ago by charles

maybe we could log the first error, but not log subsequent identical errors... jch, werehamster, how does this look to you?

Changed 12 years ago by wereHamster

comment:3 Changed 12 years ago by wereHamster

Simply hiding the error will not fix the underlying bug, event_callback() will still be called dozens of times each millisecond. It looks like dht_periodic() can fail with EAGAIN (errno 11), in that case try again right away. With my patch the web UI feels much more responsive because the libevent loop isn't overloaded with events from DHT sockets.

comment:4 Changed 12 years ago by jch

I've just checked the code (in third-party/dht/dht.c), and I don't see how dht_periodic can possibly return EAGAIN. (It's certainly not supposed to -- "call me again soon" is signalled by returning 1 with tosleep set to 0.)

Which version of the DHT is that? (Check dht/CHANGES.)

--Juliusz

comment:5 Changed 12 years ago by wereHamster

15 December 2009: dht-0.13

comment:6 Changed 12 years ago by jch

I don't see any way this version could return -1 with errno being EAGAIN.

Charles, any ideas?

--Juliusz

comment:7 Changed 12 years ago by wereHamster

Don't forget, this is on OpenSolaris?, and that OS likes to screw with its users in thousand different ways. I'll add a few printf()'s to see where the errno comes from.

comment:8 Changed 12 years ago by charles

wereHamster: I don't see any way for dht_periodic() to return -1 with errno set to EAGAIN. Good luck with the printf()s..

I'd like to get this ticket resolved before 1.80... if the two of you can suss it out in the next day or two, that would be great.

comment:9 Changed 12 years ago by wereHamster

The friendly folks (*cough*) in #opensolaris suggested that it might be because third-party/dht was compiled without -D_REENTRANT. I've modified the Makefile there and the daemon has been running then whole night now without dht_periodic not even once returning with errno set to EAGAIN.

diff --git a/third-party/dht/Makefile.am b/third-party/dht/Makefile.am
index c9e6477..f69a1d7 100644
--- a/third-party/dht/Makefile.am
+++ b/third-party/dht/Makefile.am
@@ -1,3 +1,6 @@
+
+AM_CFLAGS = @PTHREAD_CFLAGS@
+
 noinst_LIBRARIES = libdht.a
 libdht_a_SOURCES = dht.c
 noinst_HEADERS = dht.h

comment:10 Changed 12 years ago by wereHamster

I think the optimization in the dht event callback (if -> while in case dht_periodic returns with EINTR) is still useful, so I added it to the new patch.

comment:11 Changed 12 years ago by charles

wereHamster: should the other third-party libraries under Transmission's configure script (libnatpmp, miniupnpc) also have @PTHREAD_CFLAGS@ in their AM_CFLAGS?

comment:12 Changed 12 years ago by wereHamster

  • Summary changed from stdout flooded with "DHT dht_periodic failed (errno = 11) (tr-dht.c:691)" to Source code needs to be compiled with -D_REENTRANT

Likely yes. It can't hurt anyway.

comment:13 Changed 12 years ago by charles

  • Milestone changed from None Set to 1.80
  • Owner set to charles
  • Status changed from new to assigned
  • Version changed from 1.76+ to 1.70

It looks harmless to me, both in code and from testing it out on Linux yesterday & overnight. I think it's safe to commit this despite the freeze.

Reassigning Version from 1.76+ to 1.70 since this is the first time we've compiled DHT with @PTHREAD_CFLAGS@... this behavior should have existed on Solaris prior to 1.76+

comment:14 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in trunk for 1.80 by r9942

Note: See TracTickets for help on using tickets.