Properly shut down the SCTP stack.

TBR phoglund@webrtc.org for the tsan_v2/suppressions.txt change.
R=ldixon@webrtc.org, pthatcher@webrtc.org
TBR=phoglund@webrtc.org
BUG=2749

Review URL: https://webrtc-codereview.appspot.com/12739004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6484 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java

@@ -525,7 +525,7 @@ public class PeerConnectionTest extends TestCase {
   private void doTest() throws Exception {
     CountDownLatch testDone = new CountDownLatch(1);
     System.gc();  // Encourage any GC-related threads to start up.
-    //TreeSet<String> threadsBeforeTest = allThreads();
+    TreeSet<String> threadsBeforeTest = allThreads();
 
     PeerConnectionFactory factory = new PeerConnectionFactory();
     // Uncomment to get ALL WebRTC tracing and SENSITIVE libjingle logging.
@@ -742,11 +742,8 @@ public class PeerConnectionTest extends TestCase {
     factory.dispose();
     System.gc();
 
-    // TODO(ldixon): the usrsctp threads are not cleaned up (issue 2749) and
+    TreeSet<String> threadsAfterTest = allThreads();
-    // caused the assert to fail. We should reenable the assert once issue 2749
+    assertEquals(threadsBeforeTest, threadsAfterTest);
-    // is fixed.
-    //TreeSet<String> threadsAfterTest = allThreads();
-    //assertEquals(threadsBeforeTest, threadsAfterTest);
     Thread.sleep(100);
   }
 

talk/libjingle_tests.gyp

@@ -287,9 +287,7 @@
         'media/base/videoengine_unittest.h',
         'media/devices/dummydevicemanager_unittest.cc',
         'media/devices/filevideocapturer_unittest.cc',
-        # TODO(jiayl): Enable the SCTP test once the memcheck and tsan bots
+        'media/sctp/sctpdataengine_unittest.cc',
-        # failures are fixed (issue 2846).
-        #'media/sctp/sctpdataengine_unittest.cc',
         'media/webrtc/webrtcpassthroughrender_unittest.cc',
         'media/webrtc/webrtcvideocapturer_unittest.cc',
         # Omitted because depends on non-open-source testdata files.

talk/media/sctp/sctpdataengine.cc

@@ -277,18 +277,20 @@ SctpDataEngine::SctpDataEngine() {
 }
 
 SctpDataEngine::~SctpDataEngine() {
-  // TODO(ldixon): There is currently a bug in teardown of usrsctp that blocks
+  usrsctp_engines_count--;
-  // indefintely if a finish call made too soon after close calls. So teardown
+  LOG(LS_VERBOSE) << "usrsctp_engines_count:" << usrsctp_engines_count;
-  // has been skipped. Once the bug is fixed, retest and enable teardown.
+
-  // Tracked in webrtc issue 2749.
+  if (usrsctp_engines_count == 0) {
-  //
+    // usrsctp_finish() may fail if it's called too soon after the channels are
-  // usrsctp_engines_count--;
+    // closed. Wait and try again until it succeeds for up to 3 seconds.
-  // LOG(LS_VERBOSE) << "usrsctp_engines_count:" << usrsctp_engines_count;
+    for (size_t i = 0; i < 300; ++i) {
-  // if (usrsctp_engines_count == 0) {
+      if (usrsctp_finish() == 0)
-  //   if (usrsctp_finish() != 0) {
+        return;
-  //     LOG(LS_WARNING) << "usrsctp_finish.";
+
-  //   }
+      talk_base::Thread::SleepMs(10);
-  // }
+    }
+    LOG(LS_ERROR) << "Failed to shutdown usrsctp.";
+  }
 }
 
 DataMediaChannel* SctpDataEngine::CreateChannel(

talk/media/sctp/sctpdataengine_unittest.cc

@@ -295,7 +295,7 @@ class SctpDataMediaChannelTest : public testing::Test,
     params.ssrc = ssrc;
 
     return chan->SendData(params, talk_base::Buffer(
-        msg.data(), msg.length()), result);
+        &msg[0], msg.length()), result);
   }
 
   bool ReceivedData(const SctpFakeDataReceiver* recv, uint32 ssrc,
@@ -364,26 +364,26 @@ TEST_F(SctpDataMediaChannelTest, SendData) {
   EXPECT_EQ(cricket::SDR_SUCCESS, result);
   EXPECT_TRUE_WAIT(ReceivedData(receiver2(), 1, "hello?"), 1000);
   LOG(LS_VERBOSE) << "recv2.received=" << receiver2()->received()
-                  << "recv2.last_params.ssrc="
+                  << ", recv2.last_params.ssrc="
                   << receiver2()->last_params().ssrc
-                  << "recv2.last_params.timestamp="
+                  << ", recv2.last_params.timestamp="
                   << receiver2()->last_params().ssrc
-                  << "recv2.last_params.seq_num="
+                  << ", recv2.last_params.seq_num="
                   << receiver2()->last_params().seq_num
-                  << "recv2.last_data=" << receiver2()->last_data();
+                  << ", recv2.last_data=" << receiver2()->last_data();
 
   LOG(LS_VERBOSE) << "chan2 sending: 'hi chan1' -----------------------------";
   ASSERT_TRUE(SendData(channel2(), 2, "hi chan1", &result));
   EXPECT_EQ(cricket::SDR_SUCCESS, result);
   EXPECT_TRUE_WAIT(ReceivedData(receiver1(), 2, "hi chan1"), 1000);
   LOG(LS_VERBOSE) << "recv1.received=" << receiver1()->received()
-                  << "recv1.last_params.ssrc="
+                  << ", recv1.last_params.ssrc="
                   << receiver1()->last_params().ssrc
-                  << "recv1.last_params.timestamp="
+                  << ", recv1.last_params.timestamp="
                   << receiver1()->last_params().ssrc
-                  << "recv1.last_params.seq_num="
+                  << ", recv1.last_params.seq_num="
                   << receiver1()->last_params().seq_num
-                  << "recv1.last_data=" << receiver1()->last_data();
+                  << ", recv1.last_data=" << receiver1()->last_data();
 }
 
 // Sends a lot of large messages at once and verifies SDR_BLOCK is returned.
@@ -398,7 +398,7 @@ TEST_F(SctpDataMediaChannelTest, SendDataBlocked) {
 
   for (size_t i = 0; i < 100; ++i) {
     channel1()->SendData(
-        params, talk_base::Buffer(buffer.data(), buffer.size()), &result);
+        params, talk_base::Buffer(&buffer[0], buffer.size()), &result);
     if (result == cricket::SDR_BLOCK)
       break;
   }

tools/valgrind-webrtc/gtest_exclude/libjingle_media_unittest.gtest-memcheck.txt

@@ -1,2 +1,5 @@
-TODO(wu): https://code.google.com/p/webrtc/issues/detail?id=2380
+#TODO(wu): https://code.google.com/p/webrtc/issues/detail?id=2380
-WebRtcVideoMediaChannelTest.TwoStreamsSendAndUnsignalledRecv
+WebRtcVideoMediaChannelTest.TwoStreamsSendAndUnsignalledRecv
+
+#TODO(jiayl): https://code.google.com/p/webrtc/issues/detail?id=3492
+SctpDataMediaChannelTest.*

tools/valgrind-webrtc/tsan_v2/suppressions.txt

@@ -31,3 +31,7 @@ race:talk/base/logging.cc
 race:talk/base/sharedexclusivelock_unittest.cc
 race:talk/base/signalthread_unittest.cc
 race:talk/base/thread.cc
+
+# third_party/usrsctp
+# TODO(jiayl): https://code.google.com/p/webrtc/issues/detail?id=3492
+race:third_party/usrsctp/usrsctplib/user_sctp_timer_iterate.c