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

fix(transport): correctly release UDS locker file #2305

Merged
merged 2 commits into from
Aug 26, 2023
Merged

fix(transport): correctly release UDS locker file #2305

merged 2 commits into from
Aug 26, 2023

Conversation

yin1999
Copy link
Contributor

@yin1999 yin1999 commented Jul 9, 2023

The unix domain socket locker file would not be released after closing listener.

That's because the context value set by the child cannot be obtained by the parent. And the current logic is a bit complex, we have to add the logic to release locker file everywhere we have used normal unix domain socket.

return nil, err
}
ctx = context.WithValue(ctx, address, locker)
// combine listener and unix domain socket locker
defer func(locker *FileLocker) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请换成更直观的写法(非 defer)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改成了使用回调函数的方法

@yin1999 yin1999 requested a review from RPRX July 21, 2023 16:08
Comment on lines -61 to -77
if s := strings.Split(address, ","); len(s) == 2 {
address = s[0]
perm, perr := strconv.ParseUint(s[1], 8, 32)
if perr != nil {
return nil, newError("failed to parse permission: " + s[1]).Base(perr)
}

defer func(file string, permission os.FileMode) {
if err == nil {
cerr := os.Chmod(address, permission)
if cerr != nil {
err = newError("failed to set permission for " + file).Base(cerr)
}
}
}(address, os.FileMode(perm))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

将逻辑移至常规 UDS 下,抽象 UDS 没有文件系统的权限概念

@RPRX RPRX merged commit 554f4ef into XTLS:main Aug 26, 2023
35 checks passed
@RPRX
Copy link
Member

RPRX commented Aug 26, 2023

感谢 PR,目测没有明显问题,反正发版前会让大家测试下有没有引入新 bug

@yin1999 yin1999 deleted the sock-locker branch August 26, 2023 08:35
yuhan6665 pushed a commit that referenced this pull request Aug 26, 2023
* fix(transport): correctly release UDS locker file

* use callback function to do some jobs after create listener
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants