diff --git a/src/plugins/apache-httpd/src/mod_lcgdm_disk/mod_lcgdm_disk.h b/src/plugins/apache-httpd/src/mod_lcgdm_disk/mod_lcgdm_disk.h index 42164d74..62c204d3 100644 --- a/src/plugins/apache-httpd/src/mod_lcgdm_disk/mod_lcgdm_disk.h +++ b/src/plugins/apache-httpd/src/mod_lcgdm_disk/mod_lcgdm_disk.h @@ -80,7 +80,6 @@ struct dav_resource_private size_t fsize; dmlite_location loc; dmlite_fd *fd; - int fileno; // Stores fd underlying system file descriptor int copy_already_redirected; char *namespace_path; const char *content_type; diff --git a/src/plugins/apache-httpd/src/mod_lcgdm_disk/repository.c b/src/plugins/apache-httpd/src/mod_lcgdm_disk/repository.c index 14d90453..df7a7573 100644 --- a/src/plugins/apache-httpd/src/mod_lcgdm_disk/repository.c +++ b/src/plugins/apache-httpd/src/mod_lcgdm_disk/repository.c @@ -118,10 +118,10 @@ static int dav_finish_writing(dav_resource_private* info) dmlite_put_abort(info->ctx, &info->loc); if (info->fd) { + apr_pool_cleanup_kill(info->request->connection->pool, info->fd, dav_shared_fclose); dmlite_fclose(info->fd); + info->fd = NULL; } - apr_pool_cleanup_kill(info->request->connection->pool, info->fd, dav_shared_fclose); - info->fd = NULL; apr_table_unset(info->request->connection->notes, "dav_disk_info"); return e; } @@ -333,9 +336,6 @@ static dav_error *dav_disk_internal_get_resource(request_rec *r, dmlite_fstat(info->fd, &fstat); info->fsize = fstat.st_size; - /* Store underlying file descriptor */ - info->fileno = dmlite_fileno(info->fd); - /* Keep it open as long as the connection lives */ apr_pool_pre_cleanup_register(r->connection->pool, info->fd, dav_shared_fclose); } @@ -726,31 +728,40 @@ static dav_error *dav_disk_deliver(const dav_resource *resource, apr_bucket_brigade *bb; apr_bucket *bkt; dav_resource_private *info; - - info = resource->info; - bb = apr_brigade_create(resource->pool, output->c->bucket_alloc); + int fileno; if (resource->collection) { return dav_shared_new_error(resource->info->request, NULL, HTTP_FORBIDDEN, 0, "Can not list the content of a disk"); } + info = resource->info; + bb = apr_brigade_create(resource->pool, output->c->bucket_alloc); + fileno = dmlite_fileno(info->fd); + /* Apache core dir config */ core_dir_config *coreconf = (core_dir_config *) ap_get_module_config( info->request->per_dir_config, &core_module); /* Try to see if we can use sendfile */ - if (info->fileno > -1 && coreconf->enable_sendfile == ENABLE_SENDFILE_ON) { + if (fileno > -1 && coreconf->enable_sendfile == ENABLE_SENDFILE_ON) { apr_file_t* apr_file = NULL; - if (apr_os_file_put(&apr_file, &info->fileno, + if (apr_os_file_put(&apr_file, &fileno, APR_FOPEN_READ | APR_FOPEN_SENDFILE_ENABLED, info->request->pool) != APR_SUCCESS) { return dav_shared_new_error(resource->info->request, NULL, HTTP_INTERNAL_SERVER_ERROR, 0, "Could not bind the file descriptor to the apr_file"); } - apr_pool_pre_cleanup_register(info->request->pool, apr_file, - (apr_status_t (*)(void *))apr_file_close); + // apr_file_close should be used to release apr_file, but this function + // also close associated filehandle which would be closed second time + // by dmlite_fclose (called by dav_shared_fclose registered in + // dav_disk_internal_get_resource). Function apr_os_file_put is called + // with flags that doesn't currently allocate any additioanal resources + // and it is therefore safe (but still a bit danger) not to register + // apr_file_close clenup function + //apr_pool_pre_cleanup_register(info->request->pool, apr_file, + // (apr_status_t (*)(void *))apr_file_close); /* Split in chunks sendfile can handle * Adapted from mod_xsendfile */ diff --git a/src/plugins/apache-httpd/src/mod_lcgdm_ns/delivery.c b/src/plugins/apache-httpd/src/mod_lcgdm_ns/delivery.c index a8e72b9d..613539c6 100644 --- a/src/plugins/apache-httpd/src/mod_lcgdm_ns/delivery.c +++ b/src/plugins/apache-httpd/src/mod_lcgdm_ns/delivery.c @@ -384,9 +385,10 @@ dav_error *dav_ns_deliver_virtual(const dav_resource *resource, if (rbytes < 0) { err = dav_shared_new_error(resource->info->request, NULL, HTTP_INTERNAL_SERVER_ERROR, "Could not read: %s", dmlite_ferror(fd)); - dmlite_fclose(fd); } + dmlite_fclose(fd); dmlite_location_free(resource->info->virtual_location); + resource->info->virtual_location = NULL; return err; }