Skip to main content ↑ ↓

Notice

Please note that most of the software linked on this forum is likely to be safe to use. If you are unsure, feel free to ask in the relevant topics, or send a private message to an administrator or moderator. To help curb the problems of false positives, or in the event that you do find actual malware, you can contribute through the article linked here.
Topic: Foobar.exe's abort_callback in simple_thread_task causing crashes (Read 2071 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

Foobar.exe's abort_callback in simple_thread_task causing crashes

So I and my users are encountering an intermittent crash that appears to be some kind of timing/race condition thing when aborting searches in my foo_musicbrainz component.

I first query musicbrainz to get a list of matching results, and then I spawn a number of simple_thread_task's, one for each result which does the individual queries. Every one of these threads is spawned from a threaded_process_callback which gets a abort_callback& from foobar and passes it down to all the simple_thread_tasks.

Occasionally, when a user aborts a search I will get access_violations (which crash) attempting to do anything with the abort_callback (i.e. abort->is_aborting()). Unfortunately, all VS2019 will tell me is that abort_callback comes from foobar.exe and provides no useful information, so I can't tell if the ptr is now garbage, or if some value inside the object has been corrupted.

It's real intermittent so sometimes I can abort 20 times in a row and they all exit gracefully, and then other times, the first abort causes it to happen.

I'm encountering this using 1.6.5 and SDK 2021-02-23. Users have reported it in 1.6.6b7 as well.

There is certainly a chance that I'm doing something wrong, but it feels like this is an issue with foobar. @Peter, if this is something you think is worth investigating, you can get the latest beta here with the corresponding pdb.

Re: Foobar.exe's abort_callback in simple_thread_task causing crashes

Reply #1 –
You need to remember that simple_thread_task and the thread pool implementation you're using has nothing to do with the SDK/Peter. It was never part of foo_musicbrainz when I maintained it so I assume you've copied it from foo_enhanced_playcount. Remember I told you the original implementation was from WSH panel mod. It's always been rock solid but of course aborting tasks was never attempted in any code it's been used for before.

Re: Foobar.exe's abort_callback in simple_thread_task causing crashes

Reply #2 –
Quote from: snotlicker on
You need to remember that simple_thread_task and the thread pool implementation you're using has nothing to do with the SDK/Peter. It was never part of foo_musicbrainz when I maintained it so I assume you've copied it from foo_enhanced_playcount. Remember I told you the original implementation was from WSH panel mod. It's always been rock solid but of course aborting tasks was never attempted in any code it's been used for before.
Crap, you're right. I was thinking simple_thread_task was SDK, but it's not. Maybe that changes things, maybe it doesn't though.

Every one of the threads has a reference to the same abort_callback value, and none of them alter it in any way (they can't because abort_callback does not contain any methods that allow for that). abort_callback's value is controlled from foobar.exe, completely outside my component. Doesn't make a ton of sense, but maybe there's something going on where one of the child threads is reading the value, at the same time the main UI thread is altering the value?

This seems unlikely because I would expect the reading to be atomic, and also because before my recent change the crash could take up to a second to manifest. When I first discovered the issue the child threads were always waiting up to 1s (to avoid hammering) before calling request->run_ex(url, abort) which was where the crash was happening. After adding some abort->is_aborting() calls in an effort to avoid the crash, it just moved the crash position to those.

Re: Foobar.exe's abort_callback in simple_thread_task causing crashes

Reply #3 –
An abort_callback& that you get is ONLY VALID FOR THE DURATION OF THE CALL in which you got it.
You CANNOT make assumptions about its lifetime beyond that.
The same can be said for all objects passed by reference unless explicitly documented otherwise.

Passing it to other threads is OK - as long as you guarantee that these threads have exited by the time your method that got the abort_callback& returns.

A common practice in my current code is to pass std::shared_ptr<abort_callback_impl> to worker threads to avoid blocking waiting for them to finish. But you can't just turn an abort_callback& from calling code into that - create a new one and set it in your master thread when aborted by calling code.
Microsoft Windows: We can't script here, this is bat country.
 

Re: Foobar.exe's abort_callback in simple_thread_task causing crashes

Reply #4 –
Quote from: Peter on
Passing it to other threads is OK - as long as you guarantee that these threads have exited by the time your method that got the abort_callback& returns.
I was fairly confident I was doing exactly that, but you made me re-examine my code.

When I fire off my threads I sit and spin in the main threaded_process_callback waiting for them all to finish. However if I get an abort I stopped waiting for those threads to finish because the wait could have been several seconds (but is now essentially instant). abort_callback& was stale at that point and probably being overwritten while the threads were still finishing up processing and handling the abort_callback that was passed down to them.

Simple solution seems to be just enforcing main thread to wait for all the child threads to handle abort first, then exit. Problem appears to be solved. Thanks Peter!