> diff --git a/storage/perfschema/pfs_variable.cc b/storage/perfschema/pfs_variable.cc
> index 984b68e699b..85656532c72 100644
> --- a/storage/perfschema/pfs_variable.cc
> +++ b/storage/perfschema/pfs_variable.cc
> @@ -100,6 +102,15 @@ bool PFS_system_variable_cache::init_show_var_array(enum_var_type scope, bool st
> 
>    mysql_prlock_unlock(&LOCK_system_variables_hash);
> 
> +  for (uint i= 0; i < m_show_var_array.elements(); i++)

why separately, not in the loop above?

I wanted to do ti outside of LOCK_system_variables_hash, only that's why. 

> +  {
> +    sys_var *var= (sys_var *)m_show_var_array.at(i).value;
> +    if (!var) continue;

can var be NULL?

I have seen this protection elsewhere in PS code, and blindly relied on that it's important:)

See for example the loop in PFS_system_variable_cache::do_materialize_global.

They only continue until the value is NULL. Do you have a clue why?

> +    sys_var *prev= i == 0 ? NULL : (sys_var *)m_show_var_array.at(i-1).value;

hmm, normally this is done with

  sys_var *prev= NULL;

before the loop and

   prev= var;

in the loop, after plugin_lock_by_var_if_different().

Right, that'll be better. I'll rewrite. 

> @@ -316,7 +327,27 @@ class PFS_system_variable_cache_apc: public Apc_target::Apc_call
> 
>    void call_in_target_thread() override
>    {
> -    (m_pfs->*m_func)(m_param);
> +    call(m_pfs, m_func, m_param);
> +  }
> +public:
> +  static void call(PFS_system_variable_cache *pfs, Request func, uint param)
> +  {
> +    THD *safe_thd= pfs->safe_thd();
> +
> +    if (pfs->query_scope() == OPT_SESSION)

what's the point doing apc call if the query_scope() is not OPT_SESSION?
It seems like here you need a DBUG_ASSERT,
and the if() should be much earlier up the stack.

I just used the same mechanism for global variables as well. I understand now it's an overkill. Will simplify!