Re: [Tarantool-patches] [PATCH 2/6] raft: factor out the code to wakeup worker fiber

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

On 14.10.2020 15:29, Cyrill Gorcunov wrote:
> On Wed, Oct 14, 2020 at 01:28:28AM +0200, Vladislav Shpilevoy wrote:
>> -raft_schedule_broadcast(void)
>> +raft_worker_wakeup(void)
>>  {
>> -	raft.is_broadcast_scheduled = true;
>> +	if (raft.worker == NULL) {
>> +		raft.worker = fiber_new("raft_worker", raft_worker_f);
>> +		fiber_set_joinable(raft.worker, true);
>> +	}
> 
> When fiber_new return NULL you'll get nil dereference in fiber_set_joinable.

Indeed, you are right! It seems it was there always. There is no a
beautiful way to fix it now, so I added a panic() here in case of OOM:

====================
--- a/src/box/raft.c
+++ b/src/box/raft.c
@@ -1010,6 +1010,21 @@ raft_worker_wakeup(void)
 {
 	if (raft.worker == NULL) {
 		raft.worker = fiber_new("raft_worker", raft_worker_f);
+		if (raft.worker == NULL) {
+			/*
+			 * XXX: should be handled properly, no need to panic.
+			 * The issue though is that most of the Raft state
+			 * machine functions are not supposed to fail, and also
+			 * they usually wakeup the fiber when their work is
+			 * finished. So it is too late to fail. On the other
+			 * hand it looks not so good to create the fiber when
+			 * Raft is initialized. Because then it will occupy
+			 * memory even if Raft is not used.
+			 */
+			diag_log();
+			panic("Could't create Raft worker fiber");
+			return;
+		}
 		fiber_set_joinable(raft.worker, true);
 	}
 	/*