From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-2.8 required=3.0 tests=AWL,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id 83C7E1F5AE for ; Sat, 25 Jul 2020 10:33:01 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id C7DA11209FC; Sat, 25 Jul 2020 19:32:27 +0900 (JST) Received: from xtrwkhkc.outbound-mail.sendgrid.net (xtrwkhkc.outbound-mail.sendgrid.net [167.89.16.28]) by neon.ruby-lang.org (Postfix) with ESMTPS id C251B1209FB for ; Sat, 25 Jul 2020 19:32:25 +0900 (JST) Received: by filterdrecv-p3las1-7754f7d4cc-l8z88 with SMTP id filterdrecv-p3las1-7754f7d4cc-l8z88-20-5F1C0A54-17 2020-07-25 10:32:52.37534428 +0000 UTC m=+2564354.636338590 Received: from herokuapp.com (unknown) by ismtpd0046p1mdw1.sendgrid.net (SG) with ESMTP id Xvx__ddqS3KnlMyxy8AHXw for ; Sat, 25 Jul 2020 10:32:52.183 +0000 (UTC) Date: Sat, 25 Jul 2020 10:32:52 +0000 (UTC) From: eregontp@gmail.com Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 75123 X-Redmine-Project: ruby-master X-Redmine-Issue-Tracker: Bug X-Redmine-Issue-Id: 17048 X-Redmine-Issue-Author: alanwu X-Redmine-Sender: Eregon 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: =?us-ascii?Q?KippOI8ZHtTweq7XfQzW93937kJ4QNWwSBuHnaMEcr2zbviMPbAHul6QyaX+=2Fv?= =?us-ascii?Q?f3yrinegIK97HMIaNrmXBfCS5YkLX+j4DqpvNg5?= =?us-ascii?Q?PubIDO7xWqxcGnW8ZOusa7iuzMfvdKcJvKkwSxX?= =?us-ascii?Q?eq3YnfnUoHrLz7wYwDrmuz+tL2dMPZg=2FdnMTKRX?= =?us-ascii?Q?cAm=2FIoIgzZx=2FJCnriCnzz7bJSrgpqMOKz4OcMLQ?= =?us-ascii?Q?BwotyKYdk8kQj57sA=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 99331 Subject: [ruby-core:99331] [Ruby master Bug#17048] Calling initialize_copy on live modules leads to crashes 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: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #17048 has been updated by Eregon (Benoit Daloze). Should we rather design a good way to allow copying but yet not have to deal with uninitialized state? Right now the only well-defined protocols for copying are * `dup = allocate, copy @ivars, initialize_dup (which calls initialize_copy)` * `clone = allocate, copy @ivars, initialize_clone (which calls initialize_copy), clone also copies extra state like frozen and the singleton class` This means some classes have to support an "unintialized state" when otherwise they would not need to. And in some cases it even means instances have to be mutable when they would otherwise not need to (e.g., MatchData, #16294). So maybe we should make Module.allocate and #initialize_copy always raise, and override `dup` and `clone` for Module? It's still unfortunate that this would mean duplicating the logic for dup/clone there. So I think a better copying protocol is warranted here as it's not just an issue for Module. Re @nobu's patch I don't like this ad-hoc condition which leaks implementation details into semantics. How about having an `initialized` flag that's set by `#initialize` and `#initialize_copy` and checked in both of these methods if we want a quick fix? ---------------------------------------- Bug #17048: Calling initialize_copy on live modules leads to crashes https://bugs.ruby-lang.org/issues/17048#change-86726 * Author: alanwu (Alan Wu) * Status: Open * Priority: Normal * ruby -v: ruby 2.8.0dev (2020-07-23T14:44:25Z master 098e8c2873) [x86_64-linux] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN ---------------------------------------- Here's a repro script ```ruby loop do m = Module.new do prepend Module.new def hello end end klass = Class.new { include m } m.send(:initialize_copy, Module.new) GC.start klass.new.hello rescue nil end ``` Here's a script that shows that it has broken semantics even when it happens to not crash. ```ruby module A end class B include A end module C Const = :C end module D Const = :D end A.send(:initialize_copy, C) p B::Const # :C, makes sense A.send(:initialize_copy, D) p B::Const # :D, makes sense A.send(:initialize_copy, Module.new) p (begin B::Const rescue NameError; 'NameError' end) # NameError, makes sense A.send(:initialize_copy, C) p B::Const # still NameErorr. Weird ``` This example shows that the problem exists [as far back as 2.0.0](https://wandbox.org/permlink/4dVDY9sNXJ803jh8). I think the easiest way to fix this is to forbid calling `:initialize_copy` on modules that have children. Another option is to try to decide on the semantics of this. Though I don't think it's worth the effort as this has been broken for a long time and people don't seem to to be using it. Thoughts? -- https://bugs.ruby-lang.org/