haskell / core-libraries-committee

95 stars 15 forks source link

Allow setNonBlockingMode to succeed on FD`s with unknown device types. #282

Open AndreasPK opened 2 weeks ago

AndreasPK commented 2 weeks ago

What

In order to fix 25199 I propose we change setNonBlockingMode such that it will succeed when called on FD's associated with a unknown device type rather than throwing an exception.

How

Allow setNonBlockingMode to succeed on FDs if they are not associated with a known device type, and expose the helper functions used to achieve this from System.Posix.Internals.

Currently setNonBlockingMode implicitly relies on all FD's being associated with a known device type, throwing an exception if an unknown device type is associated with the given FD.

After the proposed change setNonBlockingMode will simply toggle the flag as requested if the device type is unknown.

As for the precise changes there is https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13204 (currently wip but I expect little or no code changes from it's current state).

Impact assesment

This essentially restores the pre ghc-9.10 behaviour modulo the special case handling of regular files/block devices so I expect no breakage from this change.

Bodigrim commented 4 days ago

The issue @AndreasPK is resolving here is an aftermath of #166, which introduced an additional call to fdStat to determine device type. It appeared that sometimes the file descriptor does not have a POSIX device type, in which case fdStat simple throws an exception. So what the fix is essentially about is to introduce a helper function which returns Maybe IODeviceType instead of an exception and use it in setNonBlockingMode.

This essentially restores the pre ghc-9.10 behaviour modulo the special case handling of regular files/block devices so I expect no breakage from this change.

I'd frame it vice versa: this restores older behaviour only in a special case when FD has an unknown device type.


tl;dr The proposal is to use a safe function, returning Maybe, instead of unsafe one throwing an error.

If there are no additional comments, I'll open a vote soon.

AndreasPK commented 23 hours ago

If we want to include this fix in the initial 9.12 release we will need a vote quite soon. I have no further plans to modify the merge request except for rebases (see https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13204)

Bodigrim commented 17 hours ago

Dear CLC members, let's vote on on changing guts of GHC.IO.FD to use a safe function, returning Maybe, instead of unsafe one throwing an error. The implementation is linked above.

@tomjaguarpaw @mixphix @hasufell @parsonsmatt @angerman @velveteer


+1 from me.

parsonsmatt commented 17 hours ago

+1

velveteer commented 13 hours ago

+1

hasufell commented 13 hours ago

+1

tomjaguarpaw commented 3 hours ago

+1