sudorandom / fauxrpc

Easily start a fake gRPC/gRPC-Web/Connect/REST server from protobufs
https://fauxrpc.com
MIT License
75 stars 0 forks source link

Possible deadlock in `serviceRegistry.Reset` #14

Closed DazWilkin closed 1 week ago

DazWilkin commented 1 week ago

https://github.com/sudorandom/fauxrpc/blob/b4e3ecc67000ff6ee0b99acb306f264f097fa441/private/registry/registry.go#L42

Calls to RegistryService.Reset block indefinitely:

grpcurl \
-plaintext \
--import-path=${PROTOS} \
--proto=${PROTOS}/registry/v1/registry_service.proto \
localhost:6660 \
registry.v1.RegistryService/Reset

server.Reset invokes ServiceRegistry.Reset which acquires a RW-lock on the mutex. However, before the lock is released (by the defer), it invokes AddServiceFromGlobal which (when adding the WKTs) invokes registry.AddFile which attempts to acquire a R-lock on the mutex (and can't).

I debugged by adding lambda around the locks:

server.go:

func (s *server) Reset() error {
    func() {
        log.Println("server.Reset: lock")
        s.lock.Lock()
    }()
    defer func() {
        log.Println("server.Reset: unlock")
        s.lock.Unlock()
    }()
    if err := s.ServiceRegistry.Reset(); err != nil {
        return err
    }
    return s.rebuildHandlers()
}

registry.go:

func (r *serviceRegistry) Reset() error {
    func() {
        log.Println("serviceRegistry.Reset: lock")
        r.lock.Lock()
    }()
    defer func() {
        log.Println("serviceRegistry.Reset: unlock")
        r.lock.Unlock()
    }()
    r.services = map[string]protoreflect.ServiceDescriptor{}
    r.files = new(protoregistry.Files)
    r.filesOrdered = []protoreflect.FileDescriptor{}
    return AddServicesFromGlobal(r)
}
func (r *serviceRegistry) AddFile(fd protoreflect.FileDescriptor) error {
    func() {
        log.Println("serviceRegistry.AddFile: lock")
        r.lock.Lock()
    }()
    defer func() {
        log.Println("serviceRegistry.AddFile: unlock")
        r.lock.Unlock()
    }()
    slog.Debug("AddFile", "name", fd.FullName(), "path", fd.Path())
    if _, err := r.files.FindFileByPath(fd.Path()); err == nil {
        return nil
    } else if !errors.Is(err, protoregistry.NotFound) {
        return err
    }
    if err := r.files.RegisterFile(fd); err != nil {
        if strings.Contains(err.Error(), "name conflict") {
            return nil
        }
        return err
    }
    r.filesOrdered = append(r.filesOrdered, fd)

    svcs := fd.Services()
    for i := 0; i < svcs.Len(); i++ {
        svc := svcs.Get(i)
        r.addService(svc)
    }
    return nil
}

And used the above gRPCurl, the output is:

FauxRPC (dev (none) @ unknown; go1.23.2) - 0 services loaded
Listening on http://localhost:6660
OpenAPI documentation: http://localhost:6660/fauxrpc/openapi.html

Example Commands:
$ buf curl --http2-prior-knowledge http://localhost:6660 --list-methods
$ buf curl --http2-prior-knowledge http://localhost:6660/[METHOD_NAME]
Server started.
2024/11/05 12:12:22 server.Reset: lock
2024/11/05 12:12:22 serviceRegistry.Reset: lock
2024/11/05 12:12:25 serviceRegistry.AddFile: lock
sudorandom commented 1 week ago

Thanks for reporting this! I've pushed a fix up for this. Can you please see if this works better for you now? Feel free to re-open if it's still an issue