Project

General

Profile

Bug #9036

[PATCH] io.c: make IO#reopen("pathname") atomic for non-stdio

Added by Eric Wong over 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
-
ruby -v:
ruby 2.1.0dev (2013-10-20 trunk 43369) [x86_64-linux]
Backport:
2.0.0: DONE, 2.1: DONTNEED
[ruby-core:57943]

Description

Since rb_sysopen releases the GVL, calling close(fptr->fd) would
leave a window where accessing the file from another thread can hit
IOError on a closed stream

Instead, create a new, temporary FD via rb_sysopen and call
rb_cloexec_dup2 on it to atomically replace the file fptr->fd points
to. This leaves no possible window where fptr->fd is invalid to
userspace (even for any threads running w/o GVL).

Without this patch, I need to maintain separate code paths for
atomically reopening (e.g. log rotation) std{in,out,err} vs
other files in a multithreaded application.

0001-io.c-make-IO-reopen-pathname-atomic.patch Magnifier - [PATCH] io.c: make IO#reopen("pathname") atomic (1.47 KB) Eric Wong, 10/20/2013 01:16 PM

Associated revisions

Revision 43373
Added by Nobuyoshi Nakada over 2 years ago

io.c: make IO#reopen("pathname") atomic

  • io.c (rb_io_reopen): create a new, temporary FD via rb_sysopen and call rb_cloexec_dup2 on it to atomically replace the file fptr->fd points to. This leaves no possible window where fptr->fd is invalid to userspace (even for any threads running w/o GVL). based on the patch by Eric Wong normalperson@yhbt.net at . [Bug #9036]

Revision 43373
Added by Nobuyoshi Nakada over 2 years ago

io.c: make IO#reopen("pathname") atomic

  • io.c (rb_io_reopen): create a new, temporary FD via rb_sysopen and call rb_cloexec_dup2 on it to atomically replace the file fptr->fd points to. This leaves no possible window where fptr->fd is invalid to userspace (even for any threads running w/o GVL). based on the patch by Eric Wong normalperson@yhbt.net at . [Bug #9036]

Revision 47545
Added by Usaku NAKAMURA over 1 year ago

merge revision(s) 43373: [Backport #9036]

* io.c (rb_io_reopen): create a new, temporary FD via rb_sysopen and
  call rb_cloexec_dup2 on it to atomically replace the file fptr->fd
  points to.  This leaves no possible window where fptr->fd is invalid
  to userspace (even for any threads running w/o GVL).  based on the
  patch by Eric Wong <normalperson@yhbt.net> at .
  [Bug #9036]

History

#1 [ruby-core:57944] Updated by Eric Wong over 2 years ago

"normalperson (Eric Wong)" normalperson@yhbt.net wrote:

Without this patch, I need to maintain separate code paths for
atomically reopening (e.g. log rotation) std{in,out,err} vs
other files in a multithreaded application.

Separate code paths like this:
http://bogomips.org/yahns.git/patch/?id=bcb10abe5
(review greatly appreciated!, thanks!)

#2 Updated by Nobuyoshi Nakada over 2 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r43373.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


io.c: make IO#reopen("pathname") atomic

  • io.c (rb_io_reopen): create a new, temporary FD via rb_sysopen and call rb_cloexec_dup2 on it to atomically replace the file fptr->fd points to. This leaves no possible window where fptr->fd is invalid to userspace (even for any threads running w/o GVL). based on the patch by Eric Wong normalperson@yhbt.net at . [Bug #9036]

#3 [ruby-core:63949] Updated by Yui NARUSE over 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED

#4 [ruby-core:64442] Updated by Tomoyuki Chikanaga over 1 year ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONTNEED

r43373 was already included in ruby_2.1.

#5 [ruby-core:64981] Updated by Usaku NAKAMURA over 1 year ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONTNEED to 2.0.0: DONE, 2.1: DONTNEED

backported into ruby_2_0_0 at r47545.

Also available in: Atom PDF