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

should close the file handle when release the lock #6

Closed
lichunqiang opened this issue Dec 5, 2016 · 4 comments
Closed

should close the file handle when release the lock #6

lichunqiang opened this issue Dec 5, 2016 · 4 comments

Comments

@lichunqiang
Copy link

在基于文件的锁实现方法中, release 方法是不是应该得增加 fclose 文件句柄的逻辑, 以释放资源

lichunqiang added a commit to lichunqiang/simple-fork-php that referenced this issue Dec 5, 2016
lichunqiang added a commit to lichunqiang/simple-fork-php that referenced this issue Dec 5, 2016
@white-poto
Copy link
Owner

谢谢你的pr;文件句柄短管理确实不太严谨。
进程名称的修改,最初的主要考虑低版本的兼容问题以及硬编码问题。cli_set_process_title这个函数在php5.5以后才支持,这个包最少可以支持到5.3,为了这个特性放弃版本兼容会得不偿失。另外强制修改进程名称可能会造成一些潜在的运维风险,同时用户本身可能并不想修改进程名称

@lichunqiang
Copy link
Author

进程名称的修改,最初的主要考虑低版本的兼容问题以及硬编码问题

这个确实是当时我没有注意的地方, 不过既然让用户可以设置__name__ 而这个选项并没有实质性的意义, 感觉有点tricky 😢

这个包最少可以支持到5.3

还在支持 5.3 感觉业界良心 👍

同时用户本身可能并不想修改进程名称

这个就把希望修改进程名的用户给挡在门外了

ps: 😄 通过阅读代码学到了不少, 这个是主要原因

@white-poto
Copy link
Owner

white-poto commented Dec 8, 2016

给进程设置名字其实和Java的ThreadName是一样的,虽然没有实际意义,但在实际使用中可以根据进程名称对进程进行标志和分类管理。

这个包不提供进程名称的修改功能,但是用户可以通过自己安装扩展或在php5.5以后使用cli_set_process_title,用户可以有更多的选择

@white-poto
Copy link
Owner

因为推过来的pr没有通过ci测试,我按照pr进行了一些修改已经提交了。
这个issue我先关闭了,如果有问题再重新打开

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

No branches or pull requests

2 participants