- fibers.texi (Ports): New section.
- NEWS: Update.
Fixes: guile/fibers#167
Reported-by: Christopher Baines mail@cbaines.net
civodul/port-thread-safety into master
AGit
Fixes: guile/fibers#167
Reported-by: Christopher Baines mail@cbaines.net
* fibers.texi (Ports): New section. * NEWS: Update. Fixes: guile/fibers#167 Reported-by: Christopher Baines <mail@cbaines.net>
@ -1218,0 +1242,4 @@
@code{fdopen} (@pxref{Ports and File Descriptors,,, guile, GNU Guile
Reference Manual}), and arranging so that those ports are not shared
among fibers.
@end itemize
As of Guile 3.0.11, ports implemented by @code{(ice-9
suspendable-ports)} are not thread-safe@footnote{Suspendable ports are
enabled when the @code{#:install-suspendable-ports?} argument to
@code{run-fibers} is true, @pxref{Using Fibers, @code{run-fibers}}.
I think this makes it sound like ports are thread safe if you don't enable suspendable ports, which I don't think is the case. It would be clearer to say ports aren't thread safe, including when suspendable ports is in use.
Although again, I'm not sure whether "ports aren't thread safe" is a true statement. My current guess is that it's the interaction with the buffering that causes issues, and writes that are bigger than the buffer seem to be OK.
A Fibers program is multi-threaded when the @code{#:parallelism} argument
to @code{run-fibers} is greater than one.}. Reading or writing to the
same port from fibers that happen to run on different kernel threads is
thus fraught with peril: it might work, or it might lead to garbled
input or output, or to unexpected exceptions. Writing from several
fibers to the current output or current error port is one of these
situations.
In the case of fibers, with #:parallelism of 1 and preemption enabled, could you run in to issues still? I haven't tried to reproduce this, but given the code for interacting with the port buffers is in scheme, I'd think it's possible to switch between fibers both doing I/O on the same port, and end up with the same issue as multiple threads.
The solution is to arrange so that fibers do not read or write to the
same port, for instance:@itemize
@item
by delegating all input/output on a port to a dedicated fiber;
The fibers error handling uses current-error-port, so currently at least for (current-error-port) you either need to set this such that the output is hidden/handled correctly, or avoid any uncaught exceptions that could trigger this output.
There's a similar issue in the fibers web server, although that's even harder to avoid.
To me this solution doesn't seem very practical, but I'm still up for suggesting it if this is discussed and the known pitfalls are at least included.
@item
by opening new ports for the same underlying file descriptor with
@code{fdopen} (@pxref{Ports and File Descriptors,,, guile, GNU Guile
Reference Manual}), and arranging so that those ports are not shared
among fibers.
@end itemize
I can see this working, but only if you start treating fibers like threads and have just one fiber per thread, or at least one fiber doing output per thread. Having lots of fibers each with their own ports seems wasteful in terms of ports, and if you attempt to share ports between fibers that you hope will run on the same thread, I think this could be broken through work stealing between schedulers, and then you're back to the same issue of using the same port on multiple threads.
...
My current guess of how to approach this is trying to avoid the port buffering. So I've stopped using (setvbuf (current-output-port) 'line), and I'm using put-bytevector for writing output, and providing the writes are bigger than the port buffer, the bytes get written directly, skipping any contention over the buffer.
This seems to be working for me, and also seems simpler than both of the approaches above, at least in terms of handling the output and error ports for logging. Is this an approach to suggest?
Suspendable ports are evidently not thread-safe because they manipulate the port's internal data structure without any sort of locking or atomicity. See for example how put-bytevector modifies the port's buffer cursor, end, and contents like YOLO:
;; The write will fit in the buffer, but we need to shuffle the
;; already-buffered bytes (if any) down.
(set-port-buffer-cur! buf 0)
(set-port-buffer-end! buf (+ buffered count))
(bytevector-copy! bv cur bv 0 buffered)
(bytevector-copy! src start bv buffered count)
;; If the buffer completely fills, we flush.
(when (= (+ buffered count) size)
(flush-output port))))))
I agree that the solutions I propose here are not satisfactory, but the most problematic part is Fibers' own use of the current-* ports, as you write.
Perhaps we should arrange so that each kernel thread Fibers creates is given a copy (as per fdopen) of the three current-* ports?
@civodul wrote in #169 (comment):
Suspendable ports are evidently not thread-safe because they manipulate the port's internal data structure without any sort of locking or atomicity. See for example how
put-bytevectormodifies the port's buffer cursor, end, and contents like YOLO:
Right, but this wasn't my point. My point was that the same can be said for the handling of a port's buffer if you don't enable suspendable ports.
I agree that the solutions I propose here are not satisfactory, but the most problematic part is Fibers' own use of the
current-*ports, as you write.Perhaps we should arrange so that each kernel thread Fibers creates is given a copy (as per
fdopen) of the threecurrent-*ports?
I think the best this could do is reduce the chance of problems, since it would be easy to capture the ports, then have fibers move between schedulers/threads and then you end up back with the same problem. I'd assume that preemption within a single thread might cause issues, although I failed to produce a crash when I tried. But maybe it's still worth doing.
My point was that the same can be said for the handling of a port's buffer if you don't enable suspendable ports.
Are you suggest that we should write "ports are not thread-safe" instead of "suspendable ports are not thread-safe"?
I think the best this could do is reduce the chance of problems
Hmm yes, you're right.
Not sure what to do. :-/
My point was that the same can be said for the handling of a port's buffer if you don't enable suspendable ports.
Are you suggest that we should write "ports are not thread-safe" instead of "suspendable ports are not thread-safe"?
That's along the lines of what I suggested, however I went on to say that this might also be misleading, as maybe you can use ports safely if you avoid the buffering. I'm still not sure how true this last part is.
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?