apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Ruppert ...@ruppert-it.de>
Subject Re: apr_file_close()/apr_socket_close()
Date Fri, 30 Oct 2009 09:26:23 GMT
Hi Bojan,

first of all the question is if the apr_file_t handle can be safely used 
from different threads? e.g. is APR file wrapper thread safe? And I 
don't think so! Or am I wrong? If it isn't thread safe the caller is 
responsible for locking the involved operations in different threads!

If APR file wrappers should be thread safe, it has to use a mutex to 
protect its state changes...

Regards,
Stefan


Bojan Smojver wrote:
> 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?
> 


Mime
View raw message