Re: ntdll: Assert when trying to replace an exiting file descriptor in fd_cache.
Sebastian Lackner <sebastian(a)fds-team.de> writes:
The condition "prev_fd != -1" should never be true. If this is the case then something is going terrible wrong, and it is better to assert instead of just closing the file descriptor.
It can happen if the handle was closed on the server side but not yet removed from the cache. Yes, handling of that case is not ideal, but an assert is not an improvement. -- Alexandre Julliard julliard(a)winehq.org
On 04.03.2015 08:15, Alexandre Julliard wrote:
Sebastian Lackner <sebastian(a)fds-team.de> writes:
The condition "prev_fd != -1" should never be true. If this is the case then something is going terrible wrong, and it is better to assert instead of just closing the file descriptor.
It can happen if the handle was closed on the server side but not yet removed from the cache. Yes, handling of that case is not ideal, but an assert is not an improvement.
Thanks for taking a look. Hm, from which codepath can it happen that prev_fd != -1? The function add_fd_to_cache is only called from server_get_unix_fd, and immediately before that there is a call to get_cached_fd. If an fd is cached, then the whole code will never be executed (even if its probably the wrong cached file descriptor). Since its all in an uninterrupted section I also see no other reason why fd_cache should change between get_cached_fd and add_fd_to_cache. Could you explain this a bit? server_remove_fd_from_cache shouldn't cause any harm, because this function can only unset a fd_cache entry.
Sebastian Lackner <sebastian(a)fds-team.de> writes:
On 04.03.2015 08:15, Alexandre Julliard wrote:
Sebastian Lackner <sebastian(a)fds-team.de> writes:
The condition "prev_fd != -1" should never be true. If this is the case then something is going terrible wrong, and it is better to assert instead of just closing the file descriptor.
It can happen if the handle was closed on the server side but not yet removed from the cache. Yes, handling of that case is not ideal, but an assert is not an improvement.
Thanks for taking a look. Hm, from which codepath can it happen that prev_fd != -1? The function add_fd_to_cache is only called from server_get_unix_fd, and immediately before that there is a call to get_cached_fd. If an fd is cached, then the whole code will never be executed (even if its probably the wrong cached file descriptor).
Since its all in an uninterrupted section I also see no other reason why fd_cache should change between get_cached_fd and add_fd_to_cache. Could you explain this a bit? server_remove_fd_from_cache shouldn't cause any harm, because this function can only unset a fd_cache entry.
Actually you are right, this used to be possible, but with the current code it can't happen. -- Alexandre Julliard julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Sebastian Lackner