WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#11749 closed defect (bug) (fixed)

recurse_dirsize() leaves directories open

Reported by: Denis-de-Bernardy Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Multisite Keywords: has-patch
Focuses: Cc:

Description

shouldn't we close the dir handle before returning false?

Attachments (1)

11749.diff (359 bytes) - added by Denis-de-Bernardy 4 years ago.

Download all attachments as: .zip

Change History (3)

Denis-de-Bernardy4 years ago

comment:1 hakre4 years ago

Nice question. I just wondered as well and since the PHP documnetation does offer both possiblities - automatically close with the garbage collector which can be a benefit for a functions local variables ( :) ) or not for some types (persistent database connections e.g.) I thought it's a good Idea to do some tests.

Stream Details

Resource Type Name: Stream
Created By: opendir()
Used By: readdir(), rewinddir()
Destroyed By: closedir()
Definition: Dir handle

Testing Log

  1. Closing a directory handle does not destroy the actual resource but it does change the type from stream to Unknown. Since the stream is destroyed I assume that some resources are freed.
  2. Directories opened in functions do stay open even if returned from the function. So unsetting the directory handle variable does not make any difference, even not when explicitly done with = NULL or unset().
  3. Directories explicitly closed in functions will close the directory. A next call to readdir will return false and raise a warning ( Warning: readdir(): no Directory resource supplied in __FILE__ on line __LINE__ ).
  4. Directories fully read will not autmatically close the directory handle.
  5. Using closedir() on a resource other than a Directory Resource (Type Stream, Dir Handle) will raise a warning ( Warning: closedir(): __RESOURCENUMBER__ is not a valid Directory resource in __FILE__ on line __LINE__ ).
  6. I was able to create a lot of Stream Resource Handles (on the same directory). The memory usage was quite fair (45k RAM on 200k Streams) by not closing the handles.
  7. Running the same test while closing the handles did show some difference, slightly more memory was used (about 48k RAM on 200k Streams).
  8. Since 6.) and 7.) did show some differences I developed a better test w/o any output until a certain amount of iterations was achieved. You can say that there is not much difference between the two but as reported, closing directory handles does use more time and more memory but the peak usage is the same.


Suggestion

Since directory handles do not look like somehow limited there is no fear to run out of those. Since closing does actually does only produce overhead and not any benefit, it can be suggested to not close directory handles even if this sounds wired.

Testbed
PHP PHP Version 5.2.8; Windows NT 5.1 build 2600; Server API CGI/FastCGI. Used xdebug 2.0.0RC4-dev to gather memory and execution time.

comment:2 wpmuguru4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [12953]) close dir in recurse_dirsize(), props ddebernardy, fixes #11749

Note: See TracTickets for help on using tickets.