mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 11:42:45 +00:00 
			
		
		
		
	LibIPC: Close received IPC::File fd's by default unless taken
When receiving a file descriptor over IPC, the receiver must now call take_fd() on the IPC::File to take over the descriptor. Otherwise, IPC::File will close the file on destruction.
This commit is contained in:
		
							parent
							
								
									384d047e3e
								
							
						
					
					
						commit
						7f2d8e8884
					
				
					 5 changed files with 50 additions and 5 deletions
				
			
		|  | @ -78,7 +78,7 @@ static DefaultDocumentClient s_default_document_client; | ||||||
| void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message) | void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message) | ||||||
| { | { | ||||||
|     auto file = Core::File::construct(this); |     auto file = Core::File::construct(this); | ||||||
|     if (!file->open(message.file().fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) { |     if (!file->open(message.file().take_fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) { | ||||||
|         errno = file->error(); |         errno = file->error(); | ||||||
|         perror("open"); |         perror("open"); | ||||||
|         dbgln("Failed to open project file"); |         dbgln("Failed to open project file"); | ||||||
|  |  | ||||||
|  | @ -78,7 +78,7 @@ static DefaultDocumentClient s_default_document_client; | ||||||
| void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message) | void ClientConnection::handle(const Messages::LanguageServer::FileOpened& message) | ||||||
| { | { | ||||||
|     auto file = Core::File::construct(this); |     auto file = Core::File::construct(this); | ||||||
|     if (!file->open(message.file().fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) { |     if (!file->open(message.file().take_fd(), Core::IODevice::ReadOnly, Core::File::ShouldCloseFileDescriptor::Yes)) { | ||||||
|         errno = file->error(); |         errno = file->error(); | ||||||
|         perror("open"); |         perror("open"); | ||||||
|         dbgln("Failed to open project file"); |         dbgln("Failed to open project file"); | ||||||
|  |  | ||||||
|  | @ -174,7 +174,7 @@ bool Decoder::decode([[maybe_unused]] File& file) | ||||||
|         dbgln("recvfd: {}", strerror(errno)); |         dbgln("recvfd: {}", strerror(errno)); | ||||||
|         return false; |         return false; | ||||||
|     } |     } | ||||||
|     file = File(fd); |     file = File(fd, File::ConstructWithReceivedFileDescriptor); | ||||||
|     return true; |     return true; | ||||||
| #else | #else | ||||||
|     [[maybe_unused]] auto fd = m_sockfd; |     [[maybe_unused]] auto fd = m_sockfd; | ||||||
|  |  | ||||||
|  | @ -1,5 +1,6 @@ | ||||||
| /*
 | /*
 | ||||||
|  * Copyright (c) 2020, Sergey Bugaev <bugaevc@serenityos.org> |  * Copyright (c) 2020, Sergey Bugaev <bugaevc@serenityos.org> | ||||||
|  |  * Copyright (c) 2021, Andreas Kling <kling@serenityos.org> | ||||||
|  * All rights reserved. |  * All rights reserved. | ||||||
|  * |  * | ||||||
|  * Redistribution and use in source and binary forms, with or without |  * Redistribution and use in source and binary forms, with or without | ||||||
|  | @ -26,9 +27,15 @@ | ||||||
| 
 | 
 | ||||||
| #pragma once | #pragma once | ||||||
| 
 | 
 | ||||||
|  | #include <AK/Noncopyable.h> | ||||||
|  | #include <AK/StdLibExtras.h> | ||||||
|  | #include <unistd.h> | ||||||
|  | 
 | ||||||
| namespace IPC { | namespace IPC { | ||||||
| 
 | 
 | ||||||
| class File { | class File { | ||||||
|  |     AK_MAKE_NONCOPYABLE(File); | ||||||
|  | 
 | ||||||
| public: | public: | ||||||
|     // Must have a default constructor, because LibIPC
 |     // Must have a default constructor, because LibIPC
 | ||||||
|     // default-constructs arguments prior to decoding them.
 |     // default-constructs arguments prior to decoding them.
 | ||||||
|  | @ -40,10 +47,48 @@ public: | ||||||
|     { |     { | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  |     // Tagged constructor for fd's that should be closed on destruction unless take_fd() is called.
 | ||||||
|  |     enum Tag { | ||||||
|  |         ConstructWithReceivedFileDescriptor = 1, | ||||||
|  |     }; | ||||||
|  |     File(int fd, Tag) | ||||||
|  |         : m_fd(fd) | ||||||
|  |         , m_close_on_destruction(true) | ||||||
|  |     { | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     File(File&& other) | ||||||
|  |         : m_fd(exchange(other.m_fd, -1)) | ||||||
|  |         , m_close_on_destruction(exchange(other.m_close_on_destruction, false)) | ||||||
|  |     { | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     File& operator=(File&& other) | ||||||
|  |     { | ||||||
|  |         if (this != &other) { | ||||||
|  |             m_fd = exchange(other.m_fd, -1); | ||||||
|  |             m_close_on_destruction = exchange(other.m_close_on_destruction, false); | ||||||
|  |         } | ||||||
|  |         return *this; | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     ~File() | ||||||
|  |     { | ||||||
|  |         if (m_close_on_destruction && m_fd != -1) | ||||||
|  |             close(m_fd); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|     int fd() const { return m_fd; } |     int fd() const { return m_fd; } | ||||||
| 
 | 
 | ||||||
|  |     // NOTE: This is 'const' since generated IPC messages expose all parameters by const reference.
 | ||||||
|  |     [[nodiscard]] int take_fd() const | ||||||
|  |     { | ||||||
|  |         return exchange(m_fd, -1); | ||||||
|  |     } | ||||||
|  | 
 | ||||||
| private: | private: | ||||||
|     int m_fd { -1 }; |     mutable int m_fd { -1 }; | ||||||
|  |     bool m_close_on_destruction { false }; | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -59,7 +59,7 @@ RefPtr<Download> Client::start_download(const String& method, const String& url, | ||||||
|     auto download_id = response->download_id(); |     auto download_id = response->download_id(); | ||||||
|     if (download_id < 0 || !response->response_fd().has_value()) |     if (download_id < 0 || !response->response_fd().has_value()) | ||||||
|         return nullptr; |         return nullptr; | ||||||
|     auto response_fd = response->response_fd().value().fd(); |     auto response_fd = response->response_fd().value().take_fd(); | ||||||
|     auto download = Download::create_from_id({}, *this, download_id); |     auto download = Download::create_from_id({}, *this, download_id); | ||||||
|     download->set_download_fd({}, response_fd); |     download->set_download_fd({}, response_fd); | ||||||
|     m_downloads.set(download_id, download); |     m_downloads.set(download_id, download); | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling