Remove redundant messaging

[matejak]

The send_ready already performs what the removed call could aim to accomplish.

Sending those two messages in a quick succession can lead to crashes of Anaconda.

[matejak]

@poncovka could you please be so kind and check it out? We suffer from crashes on RHEL8 and RHEL9 as well, would this fix be applicable on those platforms?

[jan-cerny]

When the redundant messaging happens? What steps should I perform to trigger it and reproduce? What steps do you perform to test this PR?

[matejak]

Sorry for omitting the BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2000998

[poncovka]

Hi, this fix seems to be harmless and should be applied in any case. However, I think it just lowering the probability of the error. It doesn't fix it.

What (I think) happens:

• The Fetch button is clicked.
• An OSCAPguiWaitForDataFetchThread thread is created.
• Later the thread sets self._fetching to False and runs the code from the set_ready decorator.
• The Fetch button is clicked again.
• Since self._fetching is False, a new thread will be created.
• However, the previous thread can still run at this point.
• In that case, Anaconda will raise the "a thread with the same name already running" error.

My theory: When you removed a command from the set_ready decorator, the window between setting self._fetching to False and actually stopping the thread became smaller and harder to hit. If you put there some sleep, it should start to happen again.

Suggested fix:

• Remove self._fetching and self._fetch_flag_lock.
• Use threadMgr.get to check if there is some fetching going on.

if threadMgr.get('OSCAPguiWaitForDataFetchThread'):
   # Fetching in progress.
   ...

[matejak]

Thanks for the analysis, so in another words the thread doesn't end right after the _fetching is set to False, but it is guaranteed that if a thread of a known name exists, we are still fetching stuff.
I would drop the _fetching only as a part of a larger-scale refactoring, as touching these things without being aware of the full picture basically is asking for trouble, but extending the definition of "not fetching at the moment" seems as a really good, and perfectly doable idea.

[scrutinizer-notifier]

The inspection completed: 1 updated code elements

[poncovka]

Thanks for the analysis, so in another words the thread doesn't end right after the _fetching is set to False, but it is guaranteed that if a thread of a known name exists, we are still fetching stuff. I would drop the _fetching only as a part of a larger-scale refactoring, as touching these things without being aware of the full picture basically is asking for trouble, but extending the definition of "not fetching at the moment" seems as a really good, and perfectly doable idea.

I think it could prevent more issues in the future, but that's up to you. Looks good to me!