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

结合动态域名解析模块,在我们项目中测试发现了几个问题 // some problems in dyups #1778

Open
alexzzh opened this issue May 19, 2023 · 6 comments

Comments

@chobits
Copy link
Member

chobits commented May 28, 2023

这似乎是源码上讨论的问题,

内存泄漏,fail时需调用ngx_slab_free_locked(shpool, peer_shm[index].sockaddr)

这个逻辑是

  1. 共享内存加锁
  2. 操作共享内存
  3. 如果步骤2异常需要退出(引用代码处),则把共享内存锁摘除,防止其他进程 运行此逻辑锁死。

所以这里不确定你遇到什么问题。这个逻辑如果不解锁反而异常

@chobits
Copy link
Member

chobits commented May 28, 2023

这里严谨的需要全部加锁。但是因为估计当初逻辑是再通用硬件上操作,默认原子操作cmp和inc。这里标记TODO估计也有考虑这个。

@alexzzh
Copy link
Author

alexzzh commented May 29, 2023

这似乎是源码上讨论的问题,

内存泄漏,fail时需调用ngx_slab_free_locked(shpool, peer_shm[index].sockaddr)

这个逻辑是

  1. 共享内存加锁
  2. 操作共享内存
  3. 如果步骤2异常需要退出(引用代码处),则把共享内存锁摘除,防止其他进程 运行此逻辑锁死。

所以这里不确定你遇到什么问题。这个逻辑如果不解锁反而异常

https://github.com/alibaba/tengine/blob/master/modules/ngx_http_upstream_check_module/ngx_http_upstream_check_module.c#L1239
这里如果goto fail, 最好判断

if (peer_shm[index].sockaddr != NULL) {
       ngx_slab_free_locked(shpool, peer_shm[index].sockaddr);
 }

@alexzzh
Copy link
Author

alexzzh commented May 29, 2023

这里严谨的需要全部加锁。但是因为估计当初逻辑是再通用硬件上操作,默认原子操作cmp和inc。这里标记TODO估计也有考虑这个。

是的,但是这里ref、owner 类型并不是ngx_atomic_t类型的,代码上多处有访问共享内存不加锁的情况

@chobits
Copy link
Member

chobits commented May 29, 2023

这里严谨的需要全部加锁。但是因为估计当初逻辑是再通用硬件上操作,默认原子操作cmp和inc。这里标记TODO估计也有考虑这个。

是的,但是这里ref、owner 类型并不是ngx_atomic_t类型的,代码上多处有访问共享内存不加锁的情况

是的。这里需要一个fix。这里todo应该改掉fix

@chobits
Copy link
Member

chobits commented May 29, 2023

todo位置,或者加锁不严谨 由此引入:9210e49b0

@chobits chobits changed the title 结合动态域名解析模块,在我们项目中测试发现了几个问题 结合动态域名解析模块,在我们项目中测试发现了几个问题 // some problems in dyups May 29, 2023
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