From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Original-To: poffice@blade.nagaokaut.ac.jp Delivered-To: poffice@blade.nagaokaut.ac.jp Received: from kankan.nagaokaut.ac.jp (kankan.nagaokaut.ac.jp [133.44.2.24]) by blade.nagaokaut.ac.jp (Postfix) with ESMTP id 3FEED19E002D for ; Sat, 19 Dec 2015 23:05:55 +0900 (JST) Received: from voscc.nagaokaut.ac.jp (voscc.nagaokaut.ac.jp [133.44.1.100]) by kankan.nagaokaut.ac.jp (Postfix) with ESMTP id 507B4B5D832 for ; Sat, 19 Dec 2015 23:38:05 +0900 (JST) Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by voscc.nagaokaut.ac.jp (Postfix) with ESMTP id 1807818CC7B1 for ; Sat, 19 Dec 2015 23:38:05 +0900 (JST) Received: from [221.186.184.76] (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id 9719C1204E1; Sat, 19 Dec 2015 23:38:04 +0900 (JST) X-Original-To: ruby-core@ruby-lang.org Delivered-To: ruby-core@ruby-lang.org Received: from o2.heroku.sendgrid.net (o2.heroku.sendgrid.net [67.228.50.55]) by neon.ruby-lang.org (Postfix) with ESMTPS id 0DB911204D4 for ; Sat, 19 Dec 2015 23:38:00 +0900 (JST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sendgrid.me; h=from:to:references:subject:mime-version:content-type:content-transfer-encoding:list-id; s=smtpapi; bh=fk6RFkUhhIcQQvMnQL/nJnBFWps=; b=HGHX5nOxGB2WXpjtet YiGsVFlI7DuzLYxuRe6Q/sWTx2SD8ZyJEmwM+EkjiOyDbarC0tnvPaqYIPHIRN6M Dv1BP7pMyY1L3XHE8qrHGxcsFN147vy3e+9dgelN8jpECu8G796TTzdM49xFVJPA FjFfoEpzu0WgklkHaZB8YUBEI= Received: by filter0632p1mdw1.sendgrid.net with SMTP id filter0632p1mdw1.23280.56756BC22F 2015-12-19 14:37:54.628295942 +0000 UTC Received: from herokuapp.com (ec2-54-205-95-246.compute-1.amazonaws.com [54.205.95.246]) by ismtpd0004p1iad1.sendgrid.net (SG) with ESMTP id 3sDsQfvmRAuBFel0blFrzQ Sat, 19 Dec 2015 14:37:54.570 +0000 (UTC) Date: Sat, 19 Dec 2015 14:37:54 +0000 From: headius@headius.com To: ruby-core@ruby-lang.org Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Redmine-MailingListIntegration-Message-Ids: 46977 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 11822 X-Redmine-Issue-Author: headius X-Redmine-Issue-Assignee: ko1 X-Redmine-Sender: headius X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: ync6xU2WACa70kv/Ymy4QrNMhiuLXJG8OTL2vJD1yS4lThHkF9CmfiXWkxH1iaXhfyctKwNmqmravf bbMDxV7TYHo1IrGw7ZGbJjgWXdw30HB1evuh5JXkR/1EDkNCJmWcMljmFYyRTtV1+C+/qhWDrP+Ifq olsMWbH3ONKsP/wsYDDE1PMIU0Y/p6lx5g0krMzv4m3nqEWeYDZMZpHvSA== X-ML-Name: ruby-core X-Mail-Count: 72382 Subject: [ruby-core:72382] [Ruby trunk - Bug #11822] Semantics of Queue#pop after close are wrong X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #11822 has been updated by Charles Nutter. tldr: Atomicity can be achieved with full locking, but it prevents Queue from being lock-free in the future. #pop should have a way to indicate the queue is closed other than just returning nil. I don't see anything that prevents the current impl of #close from going into 2.3. Atomicity ----------- It is possible to implement these operations with a hard lock, but my concern is that it prevents us from using more efficient lock-free Queue implementations in the future. I have reimplemented JRuby's Queue to mimic a GVL by locking around all operations, and it is passing tests. Performance seems fine, but there were other changes that muddle measurement a bit. Queue#pop behavior on a closed queue ---------------------------------------------- I do have concerns about #pop returning nil on a closed queue, but it is not an implementation challenge...just an API challenge. The concern is this: there's no indication from #pop that the queue is closed, since nil is a valid value to push into a queue. Or put another way: it will no longer be safe to have nil be a regular value in a queue, since it could now mean the queue is closed. You will have to check #closed? every time. Perhaps there should be a way to query if the queue is closed *and* pop at the same time? Go provides a multiple assignment form of receive: x, ok = <-ch where "ok" receives a boolean indicating that the queue is closed. We don't want to make pop always return multiple values, so here's a couple ideas for how we might do this in Ruby: (Queue#pop already takes a boolean value to indicate if it should be nonblocking, so overloading Queue#pop is not an option) ```ruby q.pop!([nonblock]) # raises exception on a closed queue q.pop?(value [, nonblock]) # returns value if queue is closed (or would block?), rather than nil ``` ---------------------------------------- Bug #11822: Semantics of Queue#pop after close are wrong https://bugs.ruby-lang.org/issues/11822#change-55671 * Author: Charles Nutter * Status: Open * Priority: Normal * Assignee: Koichi Sasada * ruby -v: * Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN ---------------------------------------- Current test/ruby/thread/test_queue.rb test_close has the following assertion that seems wrong to me: ```ruby def test_close [->{Queue.new}, ->{SizedQueue.new 3}].each do |qcreate| q = qcreate.call assert_equal false, q.closed? q << :something assert_equal q, q.close assert q.closed? assert_raise_with_message(ClosedQueueError, /closed/){q << :nothing} assert_equal q.pop, :something # <<< THIS ONE assert_nil q.pop assert_nil q.pop # non-blocking assert_raise_with_message(ThreadError, /queue empty/){q.pop(non_block=true)} end end ``` Once a queue is closed, I don't think it should ever return a result anymore. The queue should be cleared and pop should always return nil. In r52691, ko1 states that "deq'ing on closed queue returns nil, always." This test does not match that behavior. -- https://bugs.ruby-lang.org/