Skip to content

Optimized race condition in StreamHandler when creating log directory under coroutine concurrency. - #7796

Merged
limingxinleo merged 3 commits into
hyperf:masterfrom
limingxinleo:3.2-logger
Aug 10, 2026
Merged

limingxinleo merged 3 commits into
hyperf:masterfrom
limingxinleo:3.2-logger

Conversation

@limingxinleo

Copy link
Copy Markdown
Member

No description provided.

@limingxinleo limingxinleo changed the title Fixed race condition in StreamHandler when creating log directory under coroutine concurrency. Optimized race condition in StreamHandler when creating log directory under coroutine concurrency. Aug 10, 2026
@limingxinleo
limingxinleo merged commit 15df775 into hyperf:master Aug 10, 2026
78 of 80 checks passed
mkdir($dir, 0777, true);
} catch (Throwable $exception) {
if (! str_contains($exception->getMessage(), 'File exists')) {
echo (string) $exception;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这里为啥要直接echo?已经抛出异常了应该让全局异常捕获吧

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

这里为啥要直接echo?已经抛出异常了应该让全局异常捕获吧

这里虽然报错但业务逻辑是正确的,不应该进入全局异常捕获

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

意图可能是在日志服务不可用的时候直接输出到进程stdout?
如果上下文有ob_get_contents之类的话,会把echo的内容捕获掉。当然下方有抛出异常,一般就直接宕掉了。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

意图可能是在日志服务不可用的时候直接输出到进程stdout? 如果上下文有ob_get_contents之类的话,会把echo的内容捕获掉。当然下方有抛出异常,一般就直接宕掉了。

它这里是协程竞态,文件已经创建了,所有不应该异常,不过你说得对,确实不应该echo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@limingxinleo 我觉得你这个echo应该是忘记删除了?

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.

3 participants