Closed lukego closed 7 years ago
There are three ways to close a file descriptor object:
This change ensures the close() system call is only called once: the first time the file descriptor object is closed in any of these ways.
This logic already existed for fd:close() and garbage collection but it was bypassed by S.close(fd). This has lead to double-close bugs in practice.
Consider this example program:
local fd = S.open("/etc/passwd") S.close(fd) collectgarbage()
On the parent commit this code would close the file descriptor twice, once explicitly and then once with the GC finalizer:
open("/etc/passwd", O_RDONLY) = 3 close(3) = 0 close(3) = -1 EBADF (Bad file descriptor)
but with this commit it will close only once:
open("/etc/passwd", O_RDONLY) = 3 close(3) = 0
This seems important because double-close bugs on file descriptors can be very difficult to diagnose. If the file descriptor number has been reused before the second close then the effect will be unpredictable.
Yes, that seems the correct fix.
There are three ways to close a file descriptor object:
This change ensures the close() system call is only called once: the first time the file descriptor object is closed in any of these ways.
This logic already existed for fd:close() and garbage collection but it was bypassed by S.close(fd). This has lead to double-close bugs in practice.
Consider this example program:
On the parent commit this code would close the file descriptor twice, once explicitly and then once with the GC finalizer:
but with this commit it will close only once:
This seems important because double-close bugs on file descriptors can be very difficult to diagnose. If the file descriptor number has been reused before the second close then the effect will be unpredictable.