Clarify concurrency of deactivate method

[BillyDM]

It would appear the spec does not forbid the deactivate method from being called concurrently with the process method. This could lead to memory safety issues if the plugin tries to deallocate resources in its deactivate method while the process method is still using those resources.

So either:
A. In the deactivate method's docs, clarify that resources should NOT be cleaned up in this method.
or
B. Require that the process and deactivate methods not be called concurrently.

[baconpaul]

certainly seems to me that deactivate should be [main_thread && active && !processing] implying you should call stop processing before decativate if you ever called processing.

lemme ask around though to see if that would get support.

[abique]

You can't call them concurrently because deactivate would make the plugin not active which would break process() requirements.

Maybe you want to read #516 as well.

[baconpaul]

right so you cant call deactivate when processing is happening. But that means deactivate is !processing.

the only way to get to !processing is call stop processing. So I think your comment in #516 matches the spec today but not its intent also.

there's nothing in the spec which would stop you from calling processing and while it is being called, call deactivate. This pattern is legal

  AUDIO             MAIN
     |               |
  processing()       |
    .           deactivate()
    .                |
  proc returns()     |
    |                |

So either we need an entire new state 'processing is not being outstanding' (which is unimplementable without a main thread spin lock and an atomic, and is needed nowhere else in the spec) or to change the spec is what most of us concluded, which is deactivate is !processing.

[baconpaul]

Also from a plugin perspective if deactivate might be called while process is running the plugin has to lock process against the main thread if deactivate frees any resources

I think the contract has to say they do not overlap. The way to do that is to stop processing!

[abique]

This pattern is legal

  AUDIO             MAIN
     |               |
  processing()       |
    .           deactivate()
    .                |
  proc returns()     |
    |                |

No this pattern isn't legal.

[baconpaul]

Why not

[baconpaul]

Like where in the spec does it say that

[baconpaul]

By the way I agree it is not legal. Deactivste is !processing. But the spec doesn’t say that now

[abique]

Thinking it is valid is equivalent to think that you can delete an object used by another thread.

[baconpaul]

No one is arguing that is good
But why doesn’t the spec make it explicit
The transition I described violates nothing in the spec
That’s bad

[baconpaul]

So if I write that as a host I will point to the spec and say “allowed”. Using the text of the spec show I’m wrong

That’s the question

[baconpaul]

Or we can modify the spec to say !processing. Or introduce a new spec concept of not-in-process and use it here and only here

But if you think it’s wrong and everyone else does let’s figure out why the spec is missing something

[edwloef]

I would argue it violates the active requirement of process, since the plugin isn't active for the full duration of the process call.

[defiantnerd]

It lead to confusion and to keep things simple and logical as possible, it should be defined that the audio thread must have completed stop_processing() in the audio thread before calling deactivate() in the main thread.

This is also logical, because every start gets a stop, every init gets a terminate. It is just a lack of documentation IMHO.
Nobody wants people to get creative and try to save some cycles.

Also (of course) process() must have been completed before calling stop_processing().

[baconpaul]

Is active false at the end of deactivate or the start? That’s where your statement leads ed. And you’ll end up with active must be true at the end of method x and processing toggles false at the start of method x

Questions like this is why I also agree with timo. !processing is a way cleaner spec for deactivate