Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004702OCamlOCaml generalpublic2009-01-25 19:422012-09-25 20:06
Reporterdb 
Assigned Toxleroy 
PrioritynormalSeverityfeatureReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.11.0 
Target VersionFixed in Version3.12.1+dev 
Summary0004702: C-threads and callbacks
DescriptionI am hacking Systhreads library to make possible Ocaml callbacks in externally created C-threads (according to PR#1784 and http://groups.google.ru/group/fa.caml/browse_thread/thread/b2629058ac0de613 [^]). My idea is to add two functions:

CAMLextern void caml_c_thread_register(void);
CAMLextern void caml_c_thread_unregister(void);

and then use them like this:

/* register the current thread in Ocaml (once) */
caml_c_thread_register(void);
...
caml_leave_blocking_section();
caml_callback(...);
caml_enter_blocking_section();
...
/* unregister the current thread before its completion */
caml_c_thread_unregister(void);

That works for me now (under Win32 yet but pthreads are also not a problem) but caml_c_thread_register() does DuplicateHandle() inside that *theoretically* can lead to an error. What is the preferable way to handle errors in Ocaml<->C interface functions? Once this become clear I can send you a patch (if you are interested).
TagsNo tags attached.
Attached Filesdiff file icon c-threads.diff [^] (8,054 bytes) 2009-03-30 21:05 [Show Content]
diff file icon systhreads.diff [^] (1,070 bytes) 2011-05-29 13:01 [Show Content]

- Relationships
related to 0001784closedxleroy Segfault in threads library 

-  Notes
(0004885)
xleroy (administrator)
2009-03-28 18:33

This looks like a very reasonable approach. At some point I considered doing the thread registration within caml_leave_blocking_section, when it sees a thread that wasn't created from Caml, but I didn't know how to do the un-registration automatically at thread exit. The API you propose is cleaner overall. A patch will be welcome if you have completed an implementation already.

Concerning error reporting, a reasonable rule of thumb is that a C interface function intended to be called from Caml raises an exception, while a function intended to be called from C either returns an error code, and it is the caller's responsibility to handle it or ignore it. So, caml_c_thread_register() should probably return an error code. The funny thing is that the Windows tradition is to return 1 on success and 0 on error, while the Unix tradition is 0 on success and a non-zero code on error. I'll let you choose the style you prefer!


(0004891)
db (reporter)
2009-03-30 21:06

OK, the Windows implementation is ready and attached. It works for me for several months. It also makes possible unloading a DLL with the embedded threaded Ocaml runtime (unpatched Ocaml crashes as the tick thread is not terminated and sigterm handler is not restored).

As for POSIX implementation - there are some issues. Does the similar fix (stopping the tick thread and restoring the handlers) is required for anyone? Then an additional synchronization for the tick thread is needed (brute-force Win32 approach won't work). And currently I have no established Linux environment where I could test the (future) patch. So it will take some time.
(0005292)
db (reporter)
2010-04-02 15:52

There was a bug in my implementation; I will post an update this weekend.
(0005293)
xleroy (administrator)
2010-04-02 17:31

FYI, I started to merge your previous patch and adapt it to POSIX threads, and ended up refactoring a lot of code between the Win32- and POSIX-specific files. The refactored code is not yet in the SVN trunk because we haven't yet decided to include it in 3.12.0, but can be found in the branch branches/newsysthreads. So maybe you want to have a look at the refactored implementation before investing more time in your patch. I'd be interested to know about the bug you found in your original implementation. Regards,

-Xavier Leroy
(0005298)
db (reporter)
2010-04-05 13:06

Well, it maybe not entirely the bug but a feature I needed for correct work of my Ocaml module.

The following scenario was triggering the problem:
1. A C process creates a separate thread, loads Ocaml DLL and initializes Ocaml runtime.
2. Some processing is done in that thread.
3. The thread is terminated on C side. caml_c_thread_unregister() should be called. No Ocaml thread exists now. curr_thread should be NULL.

The problem was that after (3) curr_thread actually contained a garbage, so Ocaml runtime crashed then another C thread was created and caml_c_thread_register() was called. And some special care should be taken to not caml_thread_scan_roots() when no Ocaml thread existed.

If you are interested in that usage pattern to work correctly I will check out the new implementation and post the necessary changes.
(0005391)
xleroy (administrator)
2010-04-27 09:59

Merged branches/newsysthreads in the SVN trunk, will go in 3.12.0.
Rewrote caml_thread_{un,}register so that they do not depend on curr_thread.
More testing is needed, both for the new registration stuff and for the refactored library in general.
(0005937)
db (reporter)
2011-05-29 12:58

Sorry for not reporting this in time, but it would be great if you do systhreads cleanup at Ocaml runtime termination. Otherwise DLL-embedded Ocaml would crash after DLL unloading. I will attach the patch if it's possible for a reopened issue.
(0005977)
xleroy (administrator)
2011-06-04 17:36

Dmitry, please send the patch (if you can't attach it, e-mail it to Xavier.Leroy AT inria.fr). I'm afraid you're the only one affected by the problem, so it won't be fixed without your help. Thanks.
(0005981)
db (reporter)
2011-06-05 12:02

I have already attached it to this issue (systhreads.diff). If something is wrong with it, please let me know.
(0005983)
xleroy (administrator)
2011-06-06 09:35

Thanks for the patch (I didn't see it was already there). Applied in 3.12 bugfix branch; will go in 3.12.1.

- Issue History
Date Modified Username Field Change
2009-01-25 19:42 db New Issue
2009-03-28 18:33 xleroy Note Added: 0004885
2009-03-28 18:33 xleroy Status new => feedback
2009-03-30 21:05 db File Added: c-threads.diff
2009-03-30 21:06 db Note Added: 0004891
2009-08-20 16:28 xclerc Relationship added related to 0001784
2010-04-02 14:03 xleroy Assigned To => xleroy
2010-04-02 15:52 db Note Added: 0005292
2010-04-02 17:31 xleroy Note Added: 0005293
2010-04-05 13:06 db Note Added: 0005298
2010-04-27 09:59 xleroy Note Added: 0005391
2010-04-27 09:59 xleroy Status feedback => resolved
2010-04-27 09:59 xleroy Resolution open => fixed
2010-04-27 09:59 xleroy Fixed in Version => 3.12.0+dev
2011-05-29 12:19 xleroy Status resolved => closed
2011-05-29 12:58 db Note Added: 0005937
2011-05-29 12:58 db Status closed => feedback
2011-05-29 12:58 db Resolution fixed => reopened
2011-05-29 13:01 db File Added: systhreads.diff
2011-06-04 17:36 xleroy Note Added: 0005977
2011-06-05 12:02 db Note Added: 0005981
2011-06-06 09:35 xleroy Note Added: 0005983
2011-06-06 09:35 xleroy Status feedback => resolved
2011-06-06 09:35 xleroy Resolution reopened => fixed
2011-06-06 09:35 xleroy Fixed in Version 3.12.0+dev => 3.12.1+dev
2012-09-25 20:06 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker