Hello! On Thu, Dec 10, 2020 at 08:24:16PM +0300, Maxim Dounin wrote:
> Hello! > > On Thu, Dec 10, 2020 at 04:04:44PM +0000, Chris Newton wrote: > > > It has been noticed that when 'of' as returned by ngx_open_cached_file() is > > not is_file, but otherwise valid and also not is_dir, then both the > > ngx_http_flv_handler() and ngx_http_mp4_handler() functions will call > > ngx_close_file() immediately. However, the ngx_pool_cleanup_file() will > > still be called, leading to a duplicate ngx_close_file() being performed. > > > > It seems that these calls to ngx_close_file() should just be removed; eg., > > with the following. > > Thanks for the report. This was an omission in the flv module > during introduction of open_file_cache in 1454:f497ed7682a7. And > later it was copied to the mp4 module. > > Indeed, removing the ngx_close_file() close call is the most > simple solution, and that's what 1454:f497ed7682a7 does in the > static module. > > # HG changeset patch > # User Maxim Dounin <[email protected]> > # Date 1607620894 -10800 > # Thu Dec 10 20:21:34 2020 +0300 > # Node ID 09b25d66cf7e8fe1dc1c521867387ee828c7245e > # Parent 2fec22332ff45b220b59e72266c5d0a622f21d15 > Fixed double close of non-regular files in flv and mp4. > > With introduction of open_file_cache in 1454:f497ed7682a7, opening a file > with ngx_open_cached_file() automatically adds a cleanup handler to close > the file. As such, calling ngx_close_file() directly for non-regular files > is no longer needed and will result in duplicate close() call. > > In 1454:f497ed7682a7 ngx_close_file() call for non-regular files was removed > in the static module, but wasn't in the flv module. And the resulting > incorrect code was later copied to the mp4 module. Fix is to remove the > ngx_close_file() call from both modules. > > Reported by Chris Newton. > > diff --git a/src/http/modules/ngx_http_flv_module.c > b/src/http/modules/ngx_http_flv_module.c > --- a/src/http/modules/ngx_http_flv_module.c > +++ b/src/http/modules/ngx_http_flv_module.c > @@ -156,12 +156,6 @@ ngx_http_flv_handler(ngx_http_request_t > } > > if (!of.is_file) { > - > - if (ngx_close_file(of.fd) == NGX_FILE_ERROR) { > - ngx_log_error(NGX_LOG_ALERT, log, ngx_errno, > - ngx_close_file_n " \"%s\" failed", path.data); > - } > - > return NGX_DECLINED; > } > > diff --git a/src/http/modules/ngx_http_mp4_module.c > b/src/http/modules/ngx_http_mp4_module.c > --- a/src/http/modules/ngx_http_mp4_module.c > +++ b/src/http/modules/ngx_http_mp4_module.c > @@ -521,12 +521,6 @@ ngx_http_mp4_handler(ngx_http_request_t > } > > if (!of.is_file) { > - > - if (ngx_close_file(of.fd) == NGX_FILE_ERROR) { > - ngx_log_error(NGX_LOG_ALERT, log, ngx_errno, > - ngx_close_file_n " \"%s\" failed", path.data); > - } > - > return NGX_DECLINED; > } > Committed after an internal review: http://hg.nginx.org/nginx/rev/7a55311b0dc3 Thanks. -- Maxim Dounin http://mdounin.ru/ _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
