13
45
Fork
You've already forked fibers
10

doc: Document lack of thread-safety for suspendable ports. #169

Open
civodul wants to merge 1 commit from civodul/port-thread-safety into master AGit
pull from: civodul/port-thread-safety
merge into: guile:master
guile:main
guile:revert-110-wip-cancel-timer-operations
guile:fdless-draft
guile:pr-60-adjust
guile:more-operations
guile:timer-wheel
guile:cross
guile:nice-dynamic-wind-experiment
  • fibers.texi (Ports): New section.
  • NEWS: Update.

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>
doc: Document lack of thread-safety for suspendable ports.
Some checks failed
GNU Guile 2.2 / build (pull_request) Successful in 3m19s
GNU Guile 2.2 / build-libevent (pull_request) Successful in 3m30s
GNU Guile 3.0 / build (pull_request) Successful in 3m47s
GNU Guile 3.0 / build-libevent (pull_request) Failing after 5m0s
80e6e4e843
* 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
Contributor
Copy link

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?

> 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?
Author
Owner
Copy link

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?

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: https://codeberg.org/guile/guile/src/commit/d902eb5b4a44c556aaae46cb43c54205a9657788/module/ice-9/suspendable-ports.scm#L388-L396 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?
Contributor
Copy link

@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-bytevector modifies 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 three current-* 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.

@civodul wrote in https://codeberg.org/guile/fibers/pulls/169#issuecomment-10088067: > 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: 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 three `current-*` 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.
Author
Owner
Copy link

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"? > I think the best this could do is reduce the chance of problems Hmm yes, you're right. Not sure what to do. :-/
Contributor
Copy link

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.

> > 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.
civodul closed this pull request 2026年03月30日 20:28:07 +02:00
civodul reopened this pull request 2026年04月05日 17:50:06 +02:00
Some checks failed
GNU Guile 2.2 / build (pull_request) Successful in 3m19s
GNU Guile 2.2 / build-libevent (pull_request) Successful in 3m30s
GNU Guile 3.0 / build (pull_request) Successful in 3m47s
GNU Guile 3.0 / build-libevent (pull_request) Failing after 5m0s
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin +refs/pull/169/head:civodul/port-thread-safety
git switch civodul/port-thread-safety

Merge

Merge the changes and update on Forgejo.
git switch master
git merge --no-ff civodul/port-thread-safety
git switch civodul/port-thread-safety
git rebase master
git switch master
git merge --ff-only civodul/port-thread-safety
git switch civodul/port-thread-safety
git rebase master
git switch master
git merge --no-ff civodul/port-thread-safety
git switch master
git merge --squash civodul/port-thread-safety
git switch master
git merge --ff-only civodul/port-thread-safety
git switch master
git merge civodul/port-thread-safety
git push origin master
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
guile/fibers!169
Reference in a new issue
guile/fibers
No description provided.
Delete branch "civodul/port-thread-safety"

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?