From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id A5B3D6EC5D; Tue, 6 Apr 2021 23:09:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A5B3D6EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617739783; bh=vmTCU2TMHeTZC2GA9h7mymvJy1V75SrK1BP+Wg3cOMk=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=FXS8bsXvtaeidCAnY/tX36Z8q0uSGkBUVsUw8NeEmjemI/ml4JSYQxaVdfKBs6Mlm V9HrFj980gRQHsG/2utKG9F9Q94omX1EhEHaj2/kVUx7yUKf3QHB7px97Juc74JuXE lxt5Kam98fDgS+g4ojEjYYOB+rAw2I4rUzgwgv40= Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 56BD66EC5D for ; Tue, 6 Apr 2021 23:09:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 56BD66EC5D Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lTs0n-0002UP-Ri; Tue, 06 Apr 2021 23:09:22 +0300 To: Cyrill Gorcunov Cc: tml References: <20210402123420.885834-1-gorcunov@gmail.com> <20210402123420.885834-5-gorcunov@gmail.com> <14b1c3fa-da30-e7f1-6d17-32e575d71272@tarantool.org> Message-ID: <7a7e6003-4aac-ac7f-228d-b6f723227ea9@tarantool.org> Date: Tue, 6 Apr 2021 22:09:20 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD912A3E3D5D4B49FC1CC94992626AE9EFE0A9192A937C653EE00894C459B0CD1B96DBB18E406EB65508E20CBDD4AE56122DD3D62A25EE111B7459A72DE8AD892F7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75DF2B1F23425CAE5EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637889C00975665ABF68638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCDC7508B97B26DEC17AD63C21DD1E23949DEE12B5EF257CF0389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C091DAD9F922AA71188941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6A50BD5087FBFCDAACC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7D283DE70228D6EEC731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A512B04D66A444ECA3FC020297AFDAE33CD184B5C275D03121D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C5CF6F4B28551E3FE3CACFB9C2FC6D5F8CB0CAAE6BFE1750DC115F69948797107950BCE410DDB7F11D7E09C32AA3244CF309EC56A2A4A944726CDC209CD3FFB139C99C45E8D137E9729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojAX27rRizpLlqu9SWklCTSA== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822FB96659EA2F6F6B1082F1EAB2EC4BB0C3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v20 4/7] box/module_cache: introduce modules subsystem X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" >>> +void >>> +module_free(void) >>> +{ >>> + mh_int_t e; >>> + >>> + mh_foreach(module_cache, e) { >>> + struct module *m = mh_strnptr_node(module_cache, e)->val; >>> + module_unload(m); >> >> 5. As I said in the previous review, it does not make much sense. >> If there are any not unloaded modules, and they try to unload later, >> they will see module_cache == NULL and will crash. >> >> Also you can't do unload here, because the module_cache itself does >> not keep any references. All the unloads must be done by the module >> objects owners. Not by module_cache on its own. For example, if there >> is a module having a single reference and used in some other subsystem, >> your unload will free it and make it memory invalid. That will crash >> in case the module owner will try to access it again. >> >> There should be a panic-check that the module cache is empty already. > > Not at all. You can exit tarantool via Ctrl+D inside console and > modules won't be empty and we should clean them up. So I can and > I should unload modules here. Vlad, this is _exit_ path called when > we're exiting tarantool. What I'm missing? Well, if there are modules in Lua, they might have more than 1 reference, and your module_unload won't free them anyway. But that does not matter much as you try to free the objects which don't belong to you. The refs do not belong to the module_cache subsystem. They belong to the callers of module_load. That is a bug. Freeing what does not belong to you. And since the Lua land is not properly terminated with freeing all the references, the only valid way you have here is not to do anything at all AFAIS. Or free the hash table + set it to NULL so it would at least crash in a sane way in case we ever start freeing all Lua refs. But under no circumstances can you unref the modules which you didn't ref. They were referenced by schema modules and box.lib modules, and therefore must be unreferenced by them.