Fix #2867

[mojl]

Description

This fixes #2867. Stop() now changes the state to closed always, and Start() checks the state twice, once at the start and once before allocating an association, and aborting in case Stop() was called during the handshake.

Testing

The only way I could test this locally is by sleeping inside Start().
Add a hook to sctptransport.go:

var sctpStartDelay = func(*SCTPTransport) {}

And call it in line 150:

sctpStartDelay(r)

And test:

func TestSCTPTransportStartStopRace(t *testing.T) {
	const startDelay = 3 * time.Second

	offerPC, answerPC, err := newPair()
	require.NoError(t, err)

	defer closePairNow(t, offerPC, answerPC)

	answerSCTP := answerPC.SCTP()
	inStart := make(chan struct{})

	sctpStartDelay = func(r *SCTPTransport) {
		if r != answerSCTP {
			return
		}

		close(inStart)
		time.Sleep(startDelay)
	}

	_, err = offerPC.CreateDataChannel("race", nil)
	require.NoError(t, err)

	require.NoError(t, signalPair(offerPC, answerPC))

	<-inStart

	require.NoError(t, answerPC.Close())

	time.Sleep(startDelay)


	// wasn't the main issue, but this passes too now
	// assert.Equal(t, SCTPTransportStateClosed, answerSCTP.State())
	assert.Nil(t, answerSCTP.association())
}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.17%. Comparing base (381746d) to head (e8b4492).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3489      +/-   ##
==========================================
- Coverage   86.18%   86.17%   -0.01%     
==========================================
  Files          81       81              
  Lines       10479    10486       +7     
==========================================
+ Hits         9031     9036       +5     
- Misses       1011     1012       +1     
- Partials      437      438       +1     

Flag	Coverage Δ
go	86.17% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JoTurk]

lgtm thank you, we just need to merge pion/sctp#473 first, to avoid creating a race.

[JoTurk]

This won't close a handshake until we merge and switch to pion/sctp#473 sorry for that, I'll try to get it merged.

[mojl]

Oh, I wasn't aware, thanks for the review :)