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

Queue+Threads+Cygwin native causes crash #3236

Closed
vicuna opened this Issue Oct 25, 2004 · 6 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

commented Oct 25, 2004

Original bug ID: 3236
Reporter: administrator
Assigned to: @dra27
Status: resolved (set by @xavierleroy on 2017-02-20T10:48:53Z)
Resolution: fixed
Priority: normal
Severity: minor
Target version: undecided
Fixed in version: 4.03.0
Category: platform support (windows, cross-compilation, etc)

Bug description

Full_Name: Matthew W. Boyd
Version: 3.08.1
OS: Windows/Cygwin
Submission from: outbound.mlog.com (216.127.133.66)

The following application will cause a core dump when compiled in the following
way (Under Cygwin):

ocamlopt -thread -o qthr unix.cmxa threads.cmxa qthr.ml

(* ---------------------------------------- *)
open Unix
open Printf
open Pervasives

let q = Queue.create ()

let try_take () =
try Queue.take q
with Queue.Empty -> 0

let rec randomaction n =
let () =
match Random.int 2 with
| 0 -> printf "Taking: %d\n" (try_take ())
| _ -> printf "Adding\n"; Queue.add n q
in
flush stdout;
randomaction (n+1)

let v =
let a = Array.create 10 (Thread.self()) in
for i = 0 to 9 do
a.(i) <- Thread.create randomaction i
done;
a
;;

let hent = Unix.gethostbyname "localhost" in
for i = 0 to 9 do
Thread.join v.(i)
done
;;
(* ---------------------------------------- *)

I noticed this while I was attempting to use a shared queue with multiple
threads.

In this sample application, the program will not crash unless I call
Unix.gethostbyname.

I also noticed that if I call Unix.gethostbyname after unblocking Sys.sigint,
the signal will be blocked again after the call to Unix.gethostbyname; however,
if I
unblock the signal after calling gethostbyname, it will remain unblocked.

I also noticed that I can prevent the segmentation fault by wrapping any Queue
function calls with Mutex locks.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 2, 2005

Comment author: administrator

Unlocked concurrent accesses to Queue are inherently unsafe. Still, can't
reproduce crash under Linux. Can reproduce it under Cygwin with and without
the
gethostbyname. gdb gives no useful context. No can do.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 11, 2012

Comment author: @damiendoligez

Just looking at the code of Queue.take, it's obvious that concurrent access to a queue can cause a crash.
This should be documented.

We also need to investigate the interference between Unix.gethostbyname and signals under Cygwin.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 24, 2013

Comment author: @damiendoligez

Added a warning at the top of queue.mli (branch 4.01, rev 13929).

Still need to investigate gethostbyname vs sigint.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 24, 2013

Comment author: @alainfrisch

Added a warning at the top of queue.mli (branch 4.01, rev 13929).

I'm all in favor of a more explicit documentation about thread-safety, but maybe thread-unsafety should be stressed at a higher level (there are much more thread-unsafe modules: Lazy, Hashtbl, Dynlink, etc...)

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 8, 2016

Comment author: @mshinwell

@dra Maybe you could take a look at the Cygwin issue?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 20, 2017

Comment author: @xavierleroy

Could not reproduce the signal/gethostbyname problem from the English description of it.

Amusingly, the new implementation of Queue introduced in 4.03 is a lot more atomic than the old one.

Closing this issue for good.

@vicuna vicuna closed this Feb 20, 2017

@vicuna vicuna added the bug label Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.