apr_file_close()/apr_socket_close()

[Bojan Smojver]

We have effectively this code closing the file in apr_file_close():
---------------
static apr_status_t file_cleanup(apr_file_t *file, int is_child)
{
    apr_status_t rv = APR_SUCCESS;

    if (close(file->filedes) == 0) {
        file->filedes = -1;
---------------

If someone calls apr_file_os_get() from another thread (whether they set
APR_FOPEN_XTHREAD or not), then may get a file->filedes which is an FD
of an already closed file. It gets worse - they may get an FD which was
reused by yet another thread. Quite dangerous.

I think it would be safer if we did this:
---------------
static apr_status_t file_cleanup(apr_file_t *file, int is_child)
{
    apr_status_t rv = APR_SUCCESS;
    int fd = file->filedes;

    file->filedes = -1;

    if (close(fd) == 0) {
        [ ... ]
    }
    else {
        [ ... ]
        file->filedes = fd; /* restore, close() was not successful */
    }
---------------

apr_socket_close() is similar.

Opinions?

-- 
Bojan