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

Graphics.close_graph crashes 64-bit Windows ports #678

Merged
merged 1 commit into from Jul 20, 2016

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Jul 9, 2016

A fix for MPR3963 was added in b147d07 (released in 4.01.0) by @damiendoligez. As far as I can see, this fix never worked: I think it's fundamentally broken and on the 32-bit Windows ports it doesn't work and on the 64-bit Windows ports, it causes Graphics.close_graph to bring down the runtime.

I believe the original intention was that closing the graphics window (via the X button) should result in any calls to Graphics.wait_next_event terminating with:

Graphics.Graphic_failure "graphic screen not opened"

but this is done by causing WM_DESTROY to raise an OCaml exception - but the window handler is in a non-OCaml thread, so it cannot possibly raise OCaml exceptions?

@dra27
Copy link
Member Author

dra27 commented Jul 10, 2016

At the moment, this implementation won't release two separate threads both blocking on Graphics.wait_next_event - only one should be released. I don't know whether that somewhat strange behaviour justifies the extra code - especially given the number of race conditions between the gr_check_open call in caml_gr_wait_event and arriving at the WaitForSingleObject call in caml_gr_wait_event_blocking!

MPR3963 incorrectly fixed in 4.01.0. Closing the graphics window will
now cause calls to Graphics.wait_next_event to terminate (which was the
original MPR) and calling Graphics.close_graph no longer crashes the
64-bit runtime.
@dra27
Copy link
Member Author

dra27 commented Jul 10, 2016

(updated Changes file)

@damiendoligez
Copy link
Member

Thanks for finding and fixing this bug!

@damiendoligez damiendoligez merged commit 40409f3 into ocaml:trunk Jul 20, 2016
damiendoligez added a commit that referenced this pull request Jul 20, 2016
Graphics.close_graph crashes 64-bit Windows ports
@damiendoligez
Copy link
Member

Rebased and merged into trunk.

@dra27
Copy link
Member Author

dra27 commented Jul 20, 2016

No worries - could the fix go in 4.03 too?

@alainfrisch
Copy link
Contributor

I don't think any bugfix release in planned on 4.03.

@dra27
Copy link
Member Author

dra27 commented Jul 20, 2016

Ah, fair enough!

@dra27 dra27 deleted the win-graphics branch July 20, 2016 11:42
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
Graphics.close_graph crashes 64-bit Windows ports
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
* Rebase

* ocamlformat

* Disable CFG tests.

* Restore ocamlcfg tests.

* Prologue

* Revert dataflow changes.

* Revert dataflow changes.

* Format

* Use the dedicated flag.

* Review

* Review

* CRs

* Review

* ocamlformat

* Arm64

* Review

* Review

* Review

* Review

* ocamlformat

* Fix review change

* Debugging tweaks.

* ocamlformat

* Review.

* ocamlformat

Co-authored-by: Greta Yorsh <gyorsh@janestreet.com>
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.

None yet

3 participants