Skip to content

Commit 85efa3d

Browse files
committed
issue: 2580679 Fix handling socket close flow
socket epoll context handling is risky area as far as there is no common locks to protect synchronization between epoll and socket. There is possible race between removal of fd from epoll (epoll_ctl(del), or epoll close) and socket close(). This change removes socket epoll context as the first step. After this socket does not have possibility to add events. As a result it is more safety to clean socket references from epfd_info internal objects. Signed-off-by: Igor Ivanov <[email protected]>
1 parent d8d7ccd commit 85efa3d

File tree

1 file changed

+24
-24
lines changed

1 file changed

+24
-24
lines changed

src/vma/iomux/epfd_info.cpp

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -397,29 +397,32 @@ int epfd_info::del_fd(int fd, bool passthrough)
397397
errno = ENOENT;
398398
return -1;
399399
}
400-
401-
if (temp_sock_fd_api && temp_sock_fd_api->get_epoll_context_fd() == m_epfd) {
400+
401+
if (fi->offloaded_index > 0) {
402+
assert(temp_sock_fd_api);
403+
assert(temp_sock_fd_api->get_epoll_context_fd() == m_epfd);
404+
405+
/* Firstly remove epoll context from socket
406+
* to avoid new events insertion into m_ready_fds queue
407+
*/
408+
unlock();
409+
m_ring_map_lock.lock();
410+
temp_sock_fd_api->remove_epoll_context(this);
411+
m_ring_map_lock.unlock();
412+
lock();
413+
402414
m_fd_offloaded_list.erase(temp_sock_fd_api);
403415
if (passthrough) {
404416
// In case the socket is not offloaded we must copy it to the non offloaded sockets map.
405417
// This can happen after bind(), listen() or accept() calls.
406418
m_fd_non_offloaded_map[fd] = *fi;
407419
m_fd_non_offloaded_map[fd].offloaded_index = -1;
408420
}
409-
} else {
410-
fd_info_map_t::iterator fd_iter = m_fd_non_offloaded_map.find(fd);
411-
if (fd_iter != m_fd_non_offloaded_map.end()) {
412-
m_fd_non_offloaded_map.erase(fd_iter);
413-
}
414-
}
415421

416-
if (temp_sock_fd_api && temp_sock_fd_api->ep_ready_fd_node.is_list_member()) {
417-
temp_sock_fd_api->m_epoll_event_flags = 0;
418-
m_ready_fds.erase(temp_sock_fd_api);
419-
}
420-
421-
// handle offloaded fds
422-
if (fi->offloaded_index > 0) {
422+
if (temp_sock_fd_api->ep_ready_fd_node.is_list_member()) {
423+
temp_sock_fd_api->m_epoll_event_flags = 0;
424+
m_ready_fds.erase(temp_sock_fd_api);
425+
}
423426

424427
//check if the index of fd, which is being removed, is the last one.
425428
//if does, it is enough to decrease the val of m_n_offloaded_fds in order
@@ -439,15 +442,12 @@ int epfd_info::del_fd(int fd, bool passthrough)
439442
}
440443

441444
--m_n_offloaded_fds;
442-
}
443-
444-
if (temp_sock_fd_api) {
445-
temp_sock_fd_api->m_fd_rec.reset();
446-
unlock();
447-
m_ring_map_lock.lock();
448-
temp_sock_fd_api->remove_epoll_context(this);
449-
m_ring_map_lock.unlock();
450-
lock();
445+
fi->reset();
446+
} else {
447+
fd_info_map_t::iterator fd_iter = m_fd_non_offloaded_map.find(fd);
448+
if (fd_iter != m_fd_non_offloaded_map.end()) {
449+
m_fd_non_offloaded_map.erase(fd_iter);
450+
}
451451
}
452452

453453
__log_func("fd %d removed from epfd %d", fd, m_epfd);

0 commit comments

Comments
 (0)