From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS33070 50.56.128.0/17 X-Spam-Status: No, score=0.5 required=3.0 tests=AWL,MSGID_FROM_MTA_HEADER, RDNS_NONE shortcircuit=no autolearn=no version=3.3.2 Path: news.gmane.org!not-for-mail From: Eric Wong Newsgroups: gmane.comp.lang.ruby.unicorn.general,gmane.comp.lang.ruby.rainbows.general Subject: [RFC] workaround reopen atomicity issues for stdio vs non-stdio Date: Sun, 20 Oct 2013 04:44:20 +0000 Message-ID: <20131020044420.GA2734@dcvr.yhbt.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1382244307 25859 80.91.229.3 (20 Oct 2013 04:45:07 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 20 Oct 2013 04:45:07 +0000 (UTC) Cc: rainbows-talk@rubyforge.org To: mongrel-unicorn@rubyforge.org Original-X-From: mongrel-unicorn-bounces@rubyforge.org Sun Oct 20 06:45:10 2013 Return-path: Envelope-to: gclrug-mongrel-unicorn@m.gmane.org X-Original-To: mongrel-unicorn@rubyforge.org Delivered-To: mongrel-unicorn@rubyforge.org Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: mongrel-unicorn@rubyforge.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: mongrel-unicorn-bounces@rubyforge.org Errors-To: mongrel-unicorn-bounces@rubyforge.org X-Broken-Reverse-DNS: no host name found for IP address 50.56.192.79 Xref: news.gmane.org gmane.comp.lang.ruby.unicorn.general:1881 gmane.comp.lang.ruby.rainbows.general:559 Archived-At: Received: from [50.56.192.79] (helo=rubyforge.org) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1VXkt3-0005lT-OL for gclrug-mongrel-unicorn@m.gmane.org; Sun, 20 Oct 2013 06:45:10 +0200 Received: from localhost.localdomain (localhost [127.0.0.1]) by rubyforge.org (Postfix) with ESMTP id B8A502E182; Sun, 20 Oct 2013 04:45:06 +0000 (UTC) Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by rubyforge.org (Postfix) with ESMTP id DAC0B2E182; Sun, 20 Oct 2013 04:44:21 +0000 (UTC) Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 9DD261F464; Sun, 20 Oct 2013 04:44:20 +0000 (UTC) In multithreaded apps, we must use dup2/dup3 with a temporary descriptor to reopen log files atomically. This is the only way to protect all concurrent userspace access to a file when reopening. ref: http://bugs.ruby-lang.org/issues/9036 ref: yahns commit bcb10abe53cfb1d6a8ef7daef59eb10ced397c8a --- Review of this patch is greatly appreciated. This doesn't affect most unicorn users unless they spawn threads themselves and write to log files in their app. This does affect Rainbows! users who configure Rainbows! to use threads, though. Also, I guess I should announce yahns on these lists for those not on ruby-talk: http://yahns.yhbt.net/README git clone git://yhbt.net/yahns - not for production, yet, but soon I can write HTTP servers all day long, really, I just can't stand web browsers :P lib/unicorn/util.rb | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/unicorn/util.rb b/lib/unicorn/util.rb index f84241c..94c4e37 100644 --- a/lib/unicorn/util.rb +++ b/lib/unicorn/util.rb @@ -39,7 +39,7 @@ module Unicorn::Util to_reopen.each do |fp| orig_st = begin fp.stat - rescue IOError, Errno::EBADF + rescue IOError, Errno::EBADF # race next end @@ -50,8 +50,28 @@ module Unicorn::Util end begin - File.open(fp.path, 'a') { |tmpfp| fp.reopen(tmpfp) } + # stdin, stdout, stderr are special. The following dance should + # guarantee there is no window where `fp' is unwritable in MRI + # (or any correct Ruby implementation). + # + # Fwiw, GVL has zero bearing here. This is tricky because of + # the unavoidable existence of stdio FILE * pointers for + # std{in,out,err} in all programs which may use the standard C library + if fp.fileno <= 2 + # We do not want to hit fclose(3)->dup(2) window for std{in,out,err} + # MRI will use freopen(3) here internally on std{in,out,err} + fp.reopen(fp.path, "a") + else + # We should not need this workaround, Ruby can be fixed: + # http://bugs.ruby-lang.org/issues/9036 + # MRI will not call call fclose(3) or freopen(3) here + # since there's no associated std{in,out,err} FILE * pointer + # This should atomically use dup3(2) (or dup2(2)) syscall + File.open(fp.path, "a") { |tmpfp| fp.reopen(tmpfp) } + end + fp.sync = true + fp.flush # IO#sync=true may not implicitly flush new_st = fp.stat # this should only happen in the master: -- Eric Wong _______________________________________________ Unicorn mailing list - mongrel-unicorn@rubyforge.org http://rubyforge.org/mailman/listinfo/mongrel-unicorn Do not quote signatures (like this one) or top post when replying