Code-Fight commented on code in PR #805:
URL:
https://github.com/apache/incubator-seata-go/pull/805#discussion_r2105723193
##########
pkg/saga/statemachine/engine/invoker/invoker.go:
##########
@@ -45,6 +62,132 @@ type ServiceInvokerManagerImpl struct {
mutex sync.Mutex
}
+type LocalServiceInvoker struct {
+ serviceRegistry map[string]interface{}
+ methodCache map[string]*reflect.Method
+ jsonParser JsonParser
+ mutex sync.RWMutex
+}
+
+func NewLocalServiceInvoker() *LocalServiceInvoker {
+ return &LocalServiceInvoker{
+ serviceRegistry: make(map[string]interface{}),
+ methodCache: make(map[string]*reflect.Method),
+ jsonParser: &DefaultJsonParser{},
+ }
+}
+
+func (l *LocalServiceInvoker) RegisterService(serviceName string, instance
interface{}) {
+ l.mutex.Lock()
+ defer l.mutex.Unlock()
+ l.serviceRegistry[serviceName] = instance
+}
+
+func (l *LocalServiceInvoker) Invoke(ctx context.Context, input []any, service
state.ServiceTaskState) ([]reflect.Value, error) {
+ serviceName := service.ServiceName()
+ instance, exists := l.serviceRegistry[serviceName]
+ if !exists {
+ return nil, fmt.Errorf("service %s not registered", serviceName)
+ }
+
+ methodName := service.ServiceMethod()
+ method, err := l.getMethod(serviceName, methodName,
service.ParameterTypes())
+ if err != nil {
+ return nil, err
+ }
+
+ params, err := l.resolveParameters(input, method.Type)
+ if err != nil {
+ return nil, err
+ }
+
+ return l.invokeMethod(instance, method, params), nil
+}
+
+func (l *LocalServiceInvoker) getMethod(serviceName, methodName string,
paramTypes []string) (*reflect.Method, error) {
+ key := fmt.Sprintf("%s.%s", serviceName, methodName)
+ l.mutex.RLock()
+ if method, ok := l.methodCache[key]; ok {
+ l.mutex.RUnlock()
+ return method, nil
+ }
+ l.mutex.RUnlock()
+
Review Comment:
the “mutex” performed RUnlock twice"
##########
pkg/saga/statemachine/engine/invoker/invoker.go:
##########
@@ -45,6 +62,132 @@ type ServiceInvokerManagerImpl struct {
mutex sync.Mutex
}
+type LocalServiceInvoker struct {
+ serviceRegistry map[string]interface{}
+ methodCache map[string]*reflect.Method
+ jsonParser JsonParser
+ mutex sync.RWMutex
+}
+
+func NewLocalServiceInvoker() *LocalServiceInvoker {
+ return &LocalServiceInvoker{
+ serviceRegistry: make(map[string]interface{}),
+ methodCache: make(map[string]*reflect.Method),
+ jsonParser: &DefaultJsonParser{},
+ }
+}
+
+func (l *LocalServiceInvoker) RegisterService(serviceName string, instance
interface{}) {
+ l.mutex.Lock()
+ defer l.mutex.Unlock()
+ l.serviceRegistry[serviceName] = instance
+}
+
+func (l *LocalServiceInvoker) Invoke(ctx context.Context, input []any, service
state.ServiceTaskState) ([]reflect.Value, error) {
+ serviceName := service.ServiceName()
+ instance, exists := l.serviceRegistry[serviceName]
+ if !exists {
+ return nil, fmt.Errorf("service %s not registered", serviceName)
+ }
+
+ methodName := service.ServiceMethod()
+ method, err := l.getMethod(serviceName, methodName,
service.ParameterTypes())
+ if err != nil {
+ return nil, err
+ }
+
+ params, err := l.resolveParameters(input, method.Type)
+ if err != nil {
+ return nil, err
+ }
+
+ return l.invokeMethod(instance, method, params), nil
+}
+
+func (l *LocalServiceInvoker) getMethod(serviceName, methodName string,
paramTypes []string) (*reflect.Method, error) {
+ key := fmt.Sprintf("%s.%s", serviceName, methodName)
+ l.mutex.RLock()
+ if method, ok := l.methodCache[key]; ok {
+ l.mutex.RUnlock()
+ return method, nil
+ }
+ l.mutex.RUnlock()
+
Review Comment:
Additionally, there is an issue with the logic above.
The defer statement only executes after the entire function has finished.
So, writing ‘defer l.mutex.RUnlock()’ directly does not release the lock, but
the subsequent code attempts to execute ‘l.mutex.Lock()’, which is problematic.
My suggestion regarding defer handling is to wrap the code block below into
a new function. Then, in the logic of your ‘getMethod’, you can directly call
this newly wrapped function. This ensures that the lock is acquired only once
and is guaranteed to be released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For additional commands, e-mail: notifications-h...@seata.apache.org
