From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (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 24D3F45C304 for ; Tue, 1 Dec 2020 01:49:56 +0300 (MSK) Date: Tue, 1 Dec 2020 01:49:56 +0300 From: Alexander Turenko Message-ID: <20201130224956.gj6zxz4vewh2hyo2@tkn_work_nb> References: <20201031162911.61876-1-sergos@tarantool.org> <7e2cf16f-f443-5a4b-d658-8d4e3ecd6f74@tarantool.org> <7DB50565-93F7-444F-9812-F73A5DBAFCA9@tarantool.org> <8e3d200a-d97c-6dd6-7f1e-d88e7f4acdac@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <8e3d200a-d97c-6dd6-7f1e-d88e7f4acdac@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2] core: handle fiber cancellation for fiber.cond List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy > > diff --git a/src/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c > > index 904a350d9..cc59eaafb 100644 > > --- a/src/lib/core/fiber_cond.c > > +++ b/src/lib/core/fiber_cond.c > > @@ -108,6 +108,11 @@ fiber_cond_wait_timeout(struct fiber_cond *c, double timeout) > > diag_set(TimedOut); > > return -1; > > } > > + if (fiber_is_cancelled()) { > > + if (diag_is_empty(diag_get())) > > + diag_set(FiberIsCancelled); > > I read your argumentation. AFAIU sounds like:"We have a lot of tests > that check an error in the diag in such case. Also, some user > applications can check the diag in a similar way. And stack diag can be used > only in the new versions of tarantool." So, it's true. > What I propose to think about: > - this is not simple logic. If a fiber has been cancelled, maybe an error > will be added to the diag, maybe no - 50/50. > - this behavior should be documented. > - ask Mons or Turenko for advice: "What do they think about it?" > All this up to you. The fiber_channel implementation (C level) convinced me. It is good to have the similar behaviour in fiber_cond and fiber_channel. I would only ask to document this subtle difference in our web documentation. Python document subtle changes (at least some of them) and it is nice for users. Example: Python 2: | Popen.send_signal(signal) | | Sends the signal `signal` to the child. Python 3: | Popen.send_signal(signal) | | Sends the signal `signal` to the child. | | Do nothing if the process completed. (It would be better to mark it with 'Since Python x.y.z', though.) That is the place, where Python 2 and Python 3 behaviour actually differs (when the popen implementation aware about a process termination, i.e. after .poll()). It is important, when you writting a code that must work on both Python 2 and Python 3: you should catch OSError here.