Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/gcache: dead lock when another GetOrSetFuncLock in former GetOrSetFuncLock's f function #4145

Open
canbusio opened this issue Feb 5, 2025 · 3 comments · May be fixed by #4153
Open

os/gcache: dead lock when another GetOrSetFuncLock in former GetOrSetFuncLock's f function #4145

canbusio opened this issue Feb 5, 2025 · 3 comments · May be fixed by #4153
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. enhancement help wanted planned This issue/proposal is planned into our next steps.

Comments

@canbusio
Copy link

canbusio commented Feb 5, 2025

Go version

1.23.4

GoFrame version

2.8.3

Can this bug be reproduced with the latest release?

Yes

What did you do?

demo codes:

package db

import (
	"github.com/gogf/gf/v2/os/gcache"

	"context"
	"fmt"
	"time"
)

func GetTestCached(ctx context.Context) (res *string, err error) {
	cacheKey := fmt.Sprintf("GetTest-%d", 1)

	v, err := gcache.GetOrSetFuncLock(ctx, cacheKey, GetTest, 1*time.Minute)
	if err != nil {
		return nil, err
	}

	err = v.Struct(&res)
	if err != nil {
		return nil, err
	}

	return res, nil
}

func GetTest(ctx context.Context) (value interface{}, err error) {
	var res *string
	str := "123456789"
	res = &str
	return res,nil
}

func GetTest2Cached(ctx context.Context) (res *string, err error) {
	cacheKey := fmt.Sprintf("GetTest2-%d", 1)

	v, err := gcache.GetOrSetFuncLock(ctx, cacheKey, GetTest2, 1*time.Minute) //dead lock
	//v, err := gcache.GetOrSetFunc(ctx, cacheKey, GetTest2, 1*time.Minute) //no dead lock
	if err != nil {
		return nil, err
	}

	err = v.Struct(&res)
	if err != nil {
		return nil, err
	}

	return res, nil
}

func GetTest2(ctx context.Context) (value interface{}, err error) {
	var res *string
	res,err = GetTestCached(ctx)

	return res,nil
}

What did you see happen?

dead lock when calling GetTest2Cached

What did you expect to see?

I need to get some cached data as db query params, then save results in another cache key.
both need to use GetOrSetFuncLock besides GetOrSetFunc.

@canbusio canbusio added the bug It is confirmed a bug, but don't worry, we'll handle it. label Feb 5, 2025
@canbusio
Copy link
Author

canbusio commented Feb 6, 2025

I have read related codes, and found that this is not a bug.
The lock seems to be on global cache rather than on different cache keys.
So I should use GetOrSetFunc instead.

Is it possible to lock function call by different cache keys?

@gqcn
Copy link
Member

gqcn commented Feb 13, 2025

@canbusio I see, it is a FAQ, but I think it should be improved. Since the context parameter can be recursively passed down, we can consider writing the lock identifier into the context. In subsequent recursive executions of cached functions, we can easily determine that since the current execution is already locked, there's no need to apply the lock again.

@gqcn gqcn added help wanted planned This issue/proposal is planned into our next steps. labels Feb 13, 2025
Copy link
Contributor

Hello @canbusio. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it!
你好 @canbusio。我们喜欢您的提案/反馈,并希望您或其他社区成员通过拉取请求做出贡献。我们提前感谢您的贡献,并期待对其进行审查。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It is confirmed a bug, but don't worry, we'll handle it. enhancement help wanted planned This issue/proposal is planned into our next steps.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants