lib9p

commit 5b7c15103632e9d8c9e2fdd54fc9d678f05b9d6f
parent c897af242bb0e837ad551f4431bf268fbfeac9c1
Author: Matsuda Kenji <info@mtkn.jp>
Date:   Sat, 16 Sep 2023 08:17:05 +0900

fix deadlock and rename some functions

Diffstat:
M
fid.go
 | 
27
++++++++++++++++++---------
M
req.go
 | 
33
++++++++++++++++++---------------
M
server.go
 | 
45
++++++++++++++++++++++++++-------------------

3 files changed, 62 insertions(+), 43 deletions(-)
diff --git a/fid.go b/fid.go
@@ -2,6 +2,7 @@ package lib9p
 
 import (
 	"fmt"
+	"sync"
 )
 
 type OpenMode int8 // defined by 9P
@@ -43,20 +44,28 @@ func (f *Fid) String() string {
 
 type FidPool struct {
 	m map[uint32]*Fid
+	lock *sync.Mutex
 }
 
 func allocFidPool() *FidPool {
-	f := new(FidPool)
-	f.m = make(map[uint32]*Fid)
-	return f
+	return &FidPool{
+		m: make(map[uint32]*Fid),
+		lock: new(sync.Mutex),
+	}
 }
 
 func (pool *FidPool) lookup(fid uint32) (*Fid, bool) {
+	pool.lock.Lock()
+	defer pool.lock.Unlock()
+
 	f, ok := pool.m[fid]
 	return f, ok
 }
 
 func (pool *FidPool) alloc(fid uint32) (*Fid, error) {
+	pool.lock.Lock()
+	defer pool.lock.Unlock()
+
 	if _, ok := pool.m[fid]; ok {
 		return nil, fmt.Errorf("fid already in use.")
 	}
@@ -65,16 +74,16 @@ func (pool *FidPool) alloc(fid uint32) (*Fid, error) {
 	return f, nil
 }
 
-func (pool *FidPool) delete(fid uint32) *Fid {
-	f, ok := pool.lookup(fid)
-	if !ok {
-		return nil
-	}
+func (pool *FidPool) delete(fid uint32) {
+	pool.lock.Lock()
+	defer pool.lock.Unlock()
+
 	delete(pool.m, fid)
-	return f
 }
 
 func (pool *FidPool) String() string {
+	pool.lock.Lock() // TODO: need?
+	defer pool.lock.Unlock()
 	s := "{"
 	for fnum, fstruct := range pool.m {
 		s += fmt.Sprintf(" [%d]%v", fnum, fstruct)
diff --git a/req.go b/req.go
@@ -1,7 +1,7 @@
 package lib9p
 
 import (
-	"fmt"
+	"sync"
 )
 
 type Req struct {
@@ -12,32 +12,35 @@ type Req struct {
 }
 
 type ReqPool struct {
-	m map[uint16]*Req
+	m    map[uint16]*Req
+	lock *sync.Mutex
 }
 
 func allocReqPool() *ReqPool {
-	rp := new(ReqPool)
-	rp.m = make(map[uint16]*Req)
-	return rp
+	return &ReqPool{
+		m:    make(map[uint16]*Req),
+		lock: new(sync.Mutex),
+	}
 }
 
-// AddReq allocates a Req with the specified tag in ReqPool rp.
+// Add allocates a Req with the specified tag in ReqPool rp.
 // It returns nil, ErrDupTag if there is already a Req with the specified tag.
-func (rp *ReqPool) addReq(tag uint16) (*Req, error) {
+func (rp *ReqPool) add(tag uint16) (*Req, error) {
+	rp.lock.Lock()
+	defer rp.lock.Unlock()
+
 	if _, ok := rp.m[tag]; ok {
 		return nil, ErrDupTag
 	}
-	req := new(Req)
-	req.pool = rp
+	req := &Req{
+		pool: rp,
+	}
 	rp.m[tag] = req
 	return req, nil
 }
 
-func (rp *ReqPool) deleteReq(tag uint16) error {
-	_, ok := rp.m[tag]
-	if !ok {
-		return fmt.Errorf("tag %d not found", tag)
-	}
+func (rp *ReqPool) delete(tag uint16) {
+	rp.lock.Lock()
+	defer rp.lock.Unlock()
 	delete(rp.m, tag)
-	return nil
 }
diff --git a/server.go b/server.go
@@ -7,6 +7,7 @@ import (
 	"log"
 	"os"
 	"strings"
+	"sync"
 )
 
 var chatty9P = false
@@ -16,7 +17,7 @@ func Chatty() {
 }
 
 var (
-	ErrBotch      = fmt.Errorf("botch the rock")
+	ErrBotch      = fmt.Errorf("bocchi")
 	ErrPerm       = fmt.Errorf("permission denied")
 	ErrOperation  = fmt.Errorf("operation not supported")
 	ErrDupTag     = fmt.Errorf("duplicate tag")
@@ -25,26 +26,32 @@ var (
 
 type Server struct {
 	fs     FS
-	MSize  uint32
+	mSize  uint32
 	fPool  *FidPool
 	rPool  *ReqPool
-	Reader io.Reader
-	Writer io.Writer
+	reader io.Reader
+	rlock  *sync.Mutex
+	writer io.Writer
+	wlock  *sync.Mutex
 }
 
 func NewServer(fsys FS, mSize uint32, r io.Reader, w io.Writer) *Server {
 	return &Server{
 		fs:     fsys,
-		MSize:  mSize,
+		mSize:  mSize,
 		fPool:  allocFidPool(),
 		rPool:  allocReqPool(),
-		Reader: r,
-		Writer: w,
+		reader: r,
+		rlock:  new(sync.Mutex),
+		writer: w,
+		wlock:  new(sync.Mutex),
 	}
 }
 
 func (s *Server) getReq() (*Req, error) {
-	buf, err := read9PMsg(s.Reader)
+	s.rlock.Lock()
+	buf, err := read9PMsg(s.reader)
+	s.rlock.Unlock()
 	if err != nil {
 		if err == io.EOF {
 			return nil, err
@@ -52,7 +59,7 @@ func (s *Server) getReq() (*Req, error) {
 		return nil, fmt.Errorf("read9PMsg: %v", err)
 	}
 
-	r, err := s.rPool.addReq(bufMsg(buf).Tag())
+	r, err := s.rPool.add(bufMsg(buf).Tag())
 	if err != nil {
 		log.Printf("addReq(%d): %v", bufMsg(buf).Tag(), err)
 		// duplicate tag: cons up a fake Req
@@ -95,10 +102,10 @@ func sVersion(s *Server, r *Req) {
 		version = "9P2000"
 	}
 	msize := ifcall.MSize()
-	if msize < s.MSize {
-		s.MSize = msize
+	if msize < s.mSize {
+		s.mSize = msize
 	} else {
-		msize = s.MSize
+		msize = s.mSize
 	}
 
 	ofcall := &RVersion{
@@ -283,7 +290,7 @@ func sOpen(s *Server, r *Req) {
 	r.ofcall = &ROpen{
 		tag:    ifcall.Tag(),
 		qid:    fid.Qid,
-		iounit: s.MSize - 23,
+		iounit: s.mSize - 23,
 	}
 	respond(r, nil)
 }
@@ -347,7 +354,7 @@ func sCreate(s *Server, r *Req) {
 	r.ofcall = &RCreate{
 		tag:    ifcall.Tag(),
 		qid:    fi.Qid(),
-		iounit: s.MSize - 23,
+		iounit: s.mSize - 23,
 	}
 	respond(r, nil)
 }
@@ -493,7 +500,6 @@ func sRemove(s *Server, r *Req) {
 		return
 	}
 
-	// TODO: need lock?
 	defer s.fPool.delete(ifcall.Fid())
 
 	parent, err := fid.File.Parent()
@@ -753,9 +759,10 @@ func respond(r *Req, err error) {
 		panic("ReqPool is nil but err is not EDupTag")
 	}
 	if r.pool != nil {
-		if err := r.pool.deleteReq(r.ifcall.Tag()); err != nil {
-			panic(fmt.Errorf("deleteReq: %v", err))
-		}
+		r.pool.delete(r.ifcall.Tag())
 	}
-	r.srv.Writer.Write(r.ofcall.marshal())
+
+	r.srv.wlock.Lock()
+	r.srv.writer.Write(r.ofcall.marshal())
+	r.srv.wlock.Unlock()
 }