On 26/08/22 22:00, Namhyung Kim wrote:
> On Fri, Aug 26, 2022 at 11:53 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>> Below seems adequate for now, at least logically, but maybe it
>> would confuse clang thread-safety analysis?
>
> I think it's not just about locks, the exit_browser should bail out early
> if the setup code was not called.
In those cases, use_browser is 0 or -1 unless someone has inserted
an invalid perf config like "tui.script=on" or "gtk.script=on".
So currently, in cases where exit_browser() is called without
setup_browser(), it does nothing. Which means it is only the
unconditional mutex_destroy() that needs to be conditional.
>
> Thanks,
> Namhyung
>
>
>>
>> diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
>> index 25ded88801a3..6d81be6a349e 100644
>> --- a/tools/perf/ui/setup.c
>> +++ b/tools/perf/ui/setup.c
>> @@ -73,9 +73,17 @@ int stdio__config_color(const struct option *opt __maybe_unused,
>> return 0;
>> }
>>
>> +/*
>> + * exit_browser() can get called without setup_browser() having been called
>> + * first, so it is necessary to keep track of whether ui__lock has been
>> + * initialized.
>> + */
>> +static bool ui__lock_initialized;
>> +
>> void setup_browser(bool fallback_to_pager)
>> {
>> mutex_init(&ui__lock);
>> + ui__lock_initialized = true;
>> if (use_browser < 2 && (!isatty(1) || dump_trace))
>> use_browser = 0;
>>
>> @@ -118,5 +126,6 @@ void exit_browser(bool wait_for_ok)
>> default:
>> break;
>> }
>> - mutex_destroy(&ui__lock);
>> + if (ui__lock_initialized)
>> + mutex_destroy(&ui__lock);
>> }
>>
