Fix potential deadlock on panic

internal/godartsasstesting/settings.go

@@ -0,0 +1,35 @@
+package godartsasstesting
+
+import (
+	"os"
+	"strings"
+)
+
+// IsTest reports whether we're running as a test.
+var IsTest bool
+
+func init() {
+	for _, arg := range os.Args {
+		if strings.HasPrefix(arg, "-test.") {
+			IsTest = true
+			break
+		}
+	}
+}
+
+type PanicWhen uint8
+
+func (p PanicWhen) Has(flag PanicWhen) bool {
+	return p&flag != 0
+}
+
+func (p PanicWhen) Set(flag PanicWhen) PanicWhen {
+	return p | flag
+}
+
+const (
+	// Used in tests.
+	ShouldPanicInNewCall PanicWhen = 1 << iota
+	ShouldPanicInSendInbound1
+	ShouldPanicInSendInbound2
+)

options.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/bep/godartsass/v2/internal/embeddedsass"
+	"github.com/bep/godartsass/v2/internal/godartsasstesting"
 )
 
 // Options configures a Transpiler.
@@ -144,6 +145,9 @@ type Args struct {
 
 	// Ordered list starting with options.ImportResolver, then IncludePaths.
 	sassImporters []*embeddedsass.InboundMessage_CompileRequest_Importer
+
+	// Used in tests.
+	testingShouldPanicWhen godartsasstesting.PanicWhen
 }
 
 func (args *Args) init(seq uint32, opts Options) error {
@@ -251,3 +255,11 @@ func stringPointerToString(s *string) string {
 	}
 	return *s
 }
+
+// Should only be used in tests.
+func TestingApplyArgsSettings(args *Args, shouldPanicWhen godartsasstesting.PanicWhen) {
+	if !godartsasstesting.IsTest {
+		panic("TestingApplyArgsSettings should only be used in tests")
+	}
+	args.testingShouldPanicWhen = shouldPanicWhen
+}

transpiler.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cli/safeexec"
 
 	"github.com/bep/godartsass/v2/internal/embeddedsass"
+	"github.com/bep/godartsass/v2/internal/godartsasstesting"
 	"google.golang.org/protobuf/proto"
 )
 
@@ -358,7 +359,7 @@ func (t *Transpiler) input() {
 						CanonicalizeResponse: response,
 					},
 				},
-			)
+				0)
 		case *embeddedsass.OutboundMessage_ImportRequest_:
 			call := t.getCall(compilationID)
 			url := c.ImportRequest.GetUrl()
@@ -402,7 +403,7 @@ func (t *Transpiler) input() {
 						ImportResponse: response,
 					},
 				},
-			)
+				0)
 		case *embeddedsass.OutboundMessage_LogEvent_:
 			if t.opts.LogEventHandler != nil {
 				var logEvent LogEvent
@@ -471,41 +472,51 @@ func (t *Transpiler) nextSeq() uint32 {
 }
 
 func (t *Transpiler) newCall(createInbound func(seq uint32) (*embeddedsass.InboundMessage, error), args Args) (*call, error) {
-	t.mu.Lock()
-	id := t.nextSeq()
-	req, err := createInbound(id)
-	if err != nil {
-		t.mu.Unlock()
-		return nil, err
-	}
+	id, call, err := func() (uint32, *call, error) {
+		t.mu.Lock()
+		defer t.mu.Unlock()
 
-	call := &call{
-		Request:        req,
-		Done:           make(chan *call, 1),
-		importResolver: args.ImportResolver,
-	}
+		id := t.nextSeq()
+		req, err := createInbound(id)
+		if err != nil {
+			return id, nil, err
+		}
 
-	if t.shutdown || t.closing {
-		t.mu.Unlock()
-		call.Error = ErrShutdown
-		call.done()
-		return call, nil
-	}
+		// Only set in tests.
+		if args.testingShouldPanicWhen.Has(godartsasstesting.ShouldPanicInNewCall) {
+			panic("testing ShouldPanicInNewCall")
+		}
 
-	t.pending[id] = call
+		switch req.Message.(type) {
+		case *embeddedsass.InboundMessage_CompileRequest_:
+		default:
+			return id, nil, fmt.Errorf("unsupported request message type. %T", req)
+		}
 
-	t.mu.Unlock()
+		call := &call{
+			Request:        req,
+			Done:           make(chan *call, 1),
+			importResolver: args.ImportResolver,
+		}
 
-	switch call.Request.Message.(type) {
-	case *embeddedsass.InboundMessage_CompileRequest_:
-	default:
-		return nil, fmt.Errorf("unsupported request message type. %T", call.Request.Message)
+		if t.shutdown || t.closing {
+			call.Error = ErrShutdown
+			call.done()
+			return id, call, ErrShutdown
+		}
+
+		t.pending[id] = call
+
+		return id, call, nil
+	}()
+	if err != nil {
+		return nil, err
 	}
 
-	return call, t.sendInboundMessage(id, call.Request)
+	return call, t.sendInboundMessage(id, call.Request, args.testingShouldPanicWhen)
 }
 
-func (t *Transpiler) sendInboundMessage(compilationID uint32, message *embeddedsass.InboundMessage) error {
+func (t *Transpiler) sendInboundMessage(compilationID uint32, message *embeddedsass.InboundMessage, testingShouldPanicWhen godartsasstesting.PanicWhen) error {
 	t.sendMu.Lock()
 	defer t.sendMu.Unlock()
 	t.mu.Lock()
@@ -520,6 +531,11 @@ func (t *Transpiler) sendInboundMessage(compilationID uint32, message *embeddeds
 		return fmt.Errorf("failed to marshal request: %s", err)
 	}
 
+	// Only set in tests.
+	if testingShouldPanicWhen.Has(godartsasstesting.ShouldPanicInSendInbound1) {
+		panic("testing ShouldPanicInSendInbound1")
+	}
+
 	// Every message must begin with a varint indicating the length in bytes of
 	// the remaining message including the compilation ID
 	reqLen := uint64(len(out))
@@ -538,6 +554,11 @@ func (t *Transpiler) sendInboundMessage(compilationID uint32, message *embeddeds
 		return fmt.Errorf("failed to write payload: %w", err)
 	}
 
+	// Only set in tests.
+	if testingShouldPanicWhen.Has(godartsasstesting.ShouldPanicInSendInbound2) {
+		panic("testing ShouldPanicInSendInbound2")
+	}
+
 	return nil
 }
 

transpiler_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/bep/godartsass/v2"
+	"github.com/bep/godartsass/v2/internal/godartsasstesting"
 
 	qt "github.com/frankban/quicktest"
 )
@@ -238,19 +239,35 @@ func TestTranspilerParallel(t *testing.T) {
 	defer clean()
 	var wg sync.WaitGroup
 
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 20; i++ {
 		wg.Add(1)
 		go func(num int) {
 			defer wg.Done()
-			for j := 0; j < 4; j++ {
+			for j := 0; j < 8; j++ {
 				src := fmt.Sprintf(`
 $primary-color: #%03d;
 
 div { color: $primary-color; }`, num)
 
-				result, err := transpiler.Execute(godartsass.Args{Source: src})
-				c.Check(err, qt.IsNil)
-				c.Check(result.CSS, qt.Equals, fmt.Sprintf("div {\n  color: #%03d;\n}", num))
+				var panicWhen godartsasstesting.PanicWhen
+				if num == 3 {
+					panicWhen = panicWhen | godartsasstesting.ShouldPanicInSendInbound1
+				}
+				if num == 8 {
+					panicWhen = panicWhen | godartsasstesting.ShouldPanicInNewCall
+				}
+				if num == 10 {
+					panicWhen = panicWhen | godartsasstesting.ShouldPanicInSendInbound2
+				}
+				args := godartsass.Args{Source: src}
+				godartsass.TestingApplyArgsSettings(&args, panicWhen)
+				if panicWhen > 0 {
+					c.Check(func() { transpiler.Execute(args) }, qt.PanicMatches, ".*ShouldPanicIn.*")
+				} else {
+					result, err := transpiler.Execute(args)
+					c.Check(err, qt.IsNil)
+					c.Check(result.CSS, qt.Equals, fmt.Sprintf("div {\n  color: #%03d;\n}", num))
+				}
 				if c.Failed() {
 					return
 				}