Must we still disable threads on *-*-solaris*?

Back in 2005, in 8753e7ef (svn:r4383), we disabled threading on Solaris, in order to prevent some lockup bug or other. Unfortunately, back in 2005 we weren't so good at tracking bugs, so I can't easily find who reported it or how we diagnosed it.

But this is eight years later. If there was really a platform bug, surely it's gotten better by now?

We could contact one of the two or three operators whose nodes report being "on SunOS", and ask them if their nodes still work after an explicit --enable-threads , I guess.

changed milestone to %Tor: 0.2.6.x-final in legacy/trac

added 026-triaged-1 in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: 0.2.6.x-final in Legacy / Trac priority::low in Legacy / Trac regression in Legacy / Trac resolution::fixed in Legacy / Trac solaris in Legacy / Trac status::closed in Legacy / Trac tor-relay in Legacy / Trac type::defect in Legacy / Trac labels

Trac:
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final

Trac:
Keywords: tor-relay solaris deleted, tor-relay solaris 026-triaged-1 added

Implemented in 58f42007

Trac:
Status: new to closed
Resolution: N/A to fixed

I just ran into this while testing tor-alpha releases on SunOS 5.11 joyent_20150219T102159Z (Illumos).

I bisect-ed it down to commit 5b4ee475 - all builds after run without errors, but the binaries and tests coredump immediately.

I'm quite confident Illumos and/or the Joyent Smartmachine Environment support Multithreading :->

Edit: If you contact me on IRC, I can provide a testing-Environment if needed.

Trac:
Username: ruebezahl
Resolution: fixed to N/A
Status: closed to reopened

Weird! Have you been able to get a stack trace on that? I don't see how that commit could be responsible for anything SunOS-related; it's only supposed to be removing WinCE stuff.

Hi,

5b4ee475 is the last commit on which tor doesn't coredump. (as I wrote: all builds after)

thx!

Trac:
Username: ruebezahl

So, 58f42007 is the first that fails?

I wonder, is this one of those platforms that requires us to pass additional CFLAGS to the compiler to build with threads correctly?

Trac:
Keywords: tor-relay solaris 026-triaged-1 deleted, tor-relay solaris 026-triaged-1 regression added

yep, 58f42007 is the first failure-commit. I guess nobody tested this?

Trac:
Username: ruebezahl

I wonder, is this one of those platforms that requires us to pass additional CFLAGS to the compiler to build with threads correctly?

I'm guessing it could be this. My guess is that we need to include "-pthreads pthread -mt -pthread" or something like that in our CFLAGS.

In 0.2.7, we should use acx_pthread.m4. Opening a ticket for that. Do we have an easy workaround we can do in 0.2.6?

Whoops, I misread the ticket. I think just the "-pthreads" flag has a good chance of working. Let me know if that's right?

Possible patch:

diff --git a/configure.ac b/configure.ac
index e5e36d5..b28c893 100644
--- a/configure.ac
+++ b/configure.ac
@@ -366,6 +366,12 @@ if test "$LIBS" != "$saved_LIBS"; then
    have_rt=yes
 fi
 
+case $host in
+   *-*-solaris* )
+     CFLAGS="$CFLAGS -pthreads"
+     ;;
+esac
+
 AC_SEARCH_LIBS(pthread_create, [pthread])
 AC_SEARCH_LIBS(pthread_detach, [pthread])

Oh man.

I think I have it.

diff --git a/src/common/compat_pthreads.c b/src/common/compat_pthreads.c
index f4a6cad..984e4c8 100644
--- a/src/common/compat_pthreads.c
+++ b/src/common/compat_pthreads.c
@@ -279,7 +279,7 @@ tor_threads_init(void)
     pthread_mutexattr_init(&attr_recursive);
     pthread_mutexattr_settype(&attr_recursive, PTHREAD_MUTEX_RECURSIVE);
     tor_assert(0==pthread_attr_init(&attr_detached));
-    tor_assert(0==pthread_attr_setdetachstate(&attr_detached, 1));
+    tor_assert(0==pthread_attr_setdetachstate(&attr_detached, PTHREAD_CREATE_DETACHED));
     threads_initialized = 1;
     set_main_thread();
   }

Now it doesn't crash for me.

Fix in branch bug9495_redux in my public repository.

Trac:
Status: reopened to needs_review

looks trivially correct, except the commit message ends without some :)

Fixed commit message and merged to 0.2.6 and forward!

13:57 <nickm> it's a little embarassing
13:57 <nickm> I tried commenting out that line, and it worked.
13:58 <nickm> I tried forcing the alignment of the attribute, and it didn't 
              help.
13:58 <nickm> so I read the manpage.
13:58 <nickm> And saw that I was supposed to use a constant.
13:58 <nickm> So I tried using the constant, and it worked :)

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue legacy/trac#15043 (moved)

moved from legacy/trac#9495 (moved)

added Bug label and removed 1 deleted label

added Regression Relay labels and removed 1 deleted label