Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leave blocking section on callback while polling #142

Merged
merged 1 commit into from
Dec 3, 2021

Conversation

btj
Copy link
Contributor

@btj btj commented Nov 13, 2021

Fixes #141

See also verifast/verifast#252 . With this patch, the VeriFast IDE works correctly and I can no longer get it to crash.

@garrigue
Copy link
Owner

Interesting. This makes senses, but will have to be tested extensively, on Windows and Linux also.
Note that the function naming may be confusing, but caml_enter_blocking_section actually releases the ocaml global lock, and caml_leave_blocking_section, so this means that we may need to reacquire the lock when calling a callback into ML code.
This is a bit surprising, because I thought that callbacks were only occurring in places where we had already reacquired the lock. This may have been no longer the case with the switch to polling when using GtkThread.

@btj
Copy link
Contributor Author

btj commented Nov 14, 2021

Dear @garrigue, your comments make me think that you did not carefully read the linked issue. The problem is that on MacOS, "polling" actually dispatches events.

@garrigue
Copy link
Owner

OK, now I understand.
The code was assuming that polling was just polling, checking for events without calling callbacks, and MacOS breaks that.
Note that windows is also pretty strange, not allowing you to do GUI operations in another thread, so that you have to queue them using the functions in GtkThread.
Just to be sure, on which architectures did you test your code?
I do not have much time for testing, and I'm afraid to merge this without at least testing on MacOS, Linux and Windows.

@btj
Copy link
Contributor Author

btj commented Dec 3, 2021

I (briefly) tested VeriFast built using a lablgtk patched with this PR on Windows, Mac, and Linux. It worked fine. The patch is now in the VeriFast trunk (i.e. in the official nightly builds). Students will be using VeriFast over the course of this month to complete a homework, so additional testing will happen that way.

@garrigue
Copy link
Owner

garrigue commented Dec 3, 2021

Thank you. Testing, particularly on Windows, is always a major difficulty for this kind of problem, so this is very helpful.
I will merge this PR now. Tell me if any problem appears later on.

@garrigue garrigue merged commit 6f21f44 into garrigue:master Dec 3, 2021
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.

2 participants