Skip to content

Add interpreter lock release to POMDP check function#158

Closed
AlexBork wants to merge 1 commit intostormchecker:masterfrom
AlexBork:pomdp-fix
Closed

Add interpreter lock release to POMDP check function#158
AlexBork wants to merge 1 commit intostormchecker:masterfrom
AlexBork:pomdp-fix

Conversation

@AlexBork
Copy link
Copy Markdown
Contributor

@AlexBork AlexBork commented Feb 14, 2024

The POMDP check function binding is lacking the release of the global interpreter lock that allows other processes to take control of a stopped belief exploration model checker.

Thanks to @TheGreatfpmK for pointing this out.

@volkm
Copy link
Copy Markdown
Contributor

volkm commented Feb 19, 2024

Can you elaborate a bit more on the change? As far as I understand, py::gil_scoped_release>() releases the Global Interpreter Lock and therefore potentially allows other threads to access the model checker. From your description you want to achieve the opposite though?

@AlexBork AlexBork changed the title Add guard to POMDP check function Add interpreter lock release to POMDP check function Feb 19, 2024
@AlexBork
Copy link
Copy Markdown
Contributor Author

Sorry, there was a misunderstanding of the problem on my side. The problem was indeed that the model checker was keeping the interpreter lock, so other processes were not able to restart it while it was suspended. I have changed the title and description to account for that.

@volkm
Copy link
Copy Markdown
Contributor

volkm commented Feb 19, 2024

Thanks for the clarification.
We have to be careful when releasing the GIL to avoid race conditions with multiple threads. I am okay with the change for now, but maybe we should think about general handling of parallelization in the future.
Any opinion, @sjunges?

@sjunges
Copy link
Copy Markdown
Contributor

sjunges commented Apr 2, 2024

Sorry, I was not aware of this issue. I am also concerned about this, in particular as it depends on the options whether or not this is actuallly code that is meant to run in parallel.

I would be more comfortable with a function run_check or something. We could then have a more general policy that such functions that are meant to run in parallel are always starting with run_*?

@volkm
Copy link
Copy Markdown
Contributor

volkm commented Apr 1, 2025

Maybe checking for signals in pybind is an option?

@sjunges sjunges marked this pull request as draft July 30, 2025 08:25
@sjunges
Copy link
Copy Markdown
Contributor

sjunges commented Aug 20, 2025

Is there any update on this? I don't think we will merge this code here as is. Marking it for closed right now, happy to reopen though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants