www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Is synchronized(...){...} doomed to never be nothrow/ nogc?

reply Vladimir Panteleev <thecybershadow.lists gmail.com> writes:
Here is the situation, AIUI:

1. We allow users to provide their own Monitors, which hook into 
the synchronized(obj) statement.

2. These monitors' methods are unadorned (no nothrow/ nogc).

3. As a result, synchronized(obj) statements will not compile in 
nothrow/ nogc code, because we can't know that the Monitor 
implementation doesn't throw / use the GC.

4. As Matthias mentioned, fixing this was attempted before (by 
making the Monitor methods `nothrow`). However, this broke code 
(vibe.d specifically), as its implementation was actually not 
`nothrow`: https://issues.dlang.org/show_bug.cgi?id=11216

This is a problem e.g. in Druntime code, because it currently 
forces a lot of code that should be `nothrow` to not be annotated 
as such, even when the synchronization objects used use the 
standard D monitor implementation, which is actually `nothrow`.

So I guess the way forward here for the Druntime code is to 
abandon the synchronized() statement and use locks directly?
May 10 2016
next sibling parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev wrote:
 because we can't know that the Monitor implementation doesn't 
 throw / use the GC.
Why not? Surely this information is available at compile-time?
May 10 2016
parent Vladimir Panteleev <thecybershadow.lists gmail.com> writes:
On Tuesday, 10 May 2016 at 21:01:38 UTC, Jack Stouffer wrote:
 On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev 
 wrote:
 because we can't know that the Monitor implementation doesn't 
 throw / use the GC.
Why not? Surely this information is available at compile-time?
Well, not really, because it's done via an abstract interface. See Object.Monitor in object.d.
May 10 2016
prev sibling next sibling parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/10/16 1:46 PM, Vladimir Panteleev wrote:
 Here is the situation, AIUI:

 1. We allow users to provide their own Monitors, which hook into the
 synchronized(obj) statement.

 2. These monitors' methods are unadorned (no nothrow/ nogc).

 3. As a result, synchronized(obj) statements will not compile in
 nothrow/ nogc code, because we can't know that the Monitor
 implementation doesn't throw / use the GC.

 4. As Matthias mentioned, fixing this was attempted before (by making
 the Monitor methods `nothrow`). However, this broke code (vibe.d
 specifically), as its implementation was actually not `nothrow`:
 https://issues.dlang.org/show_bug.cgi?id=11216

 This is a problem e.g. in Druntime code, because it currently forces a
 lot of code that should be `nothrow` to not be annotated as such, even
 when the synchronization objects used use the standard D monitor
 implementation, which is actually `nothrow`.

 So I guess the way forward here for the Druntime code is to abandon the
 synchronized() statement and use locks directly?
Does synchronized(someMutex) go through the same mechanism? Because it shouldn't, and then we can have the compiler properly interpret what the nothrow nogc status is. I think Monitor.lock/unlock should be nothrow/ nogc. I'm not familiar with the vibe.d code, but I'd believe that if you need those to throw or allocate, then you should have to go through another mechanism. -Steve
May 10 2016
prev sibling next sibling parent Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Tuesday, May 10, 2016 17:46:17 Vladimir Panteleev via Digitalmars-d wrote:
 Here is the situation, AIUI:

 1. We allow users to provide their own Monitors, which hook into
 the synchronized(obj) statement.

 2. These monitors' methods are unadorned (no nothrow/ nogc).

 3. As a result, synchronized(obj) statements will not compile in
 nothrow/ nogc code, because we can't know that the Monitor
 implementation doesn't throw / use the GC.

 4. As Matthias mentioned, fixing this was attempted before (by
 making the Monitor methods `nothrow`). However, this broke code
 (vibe.d specifically), as its implementation was actually not
 `nothrow`: https://issues.dlang.org/show_bug.cgi?id=11216

 This is a problem e.g. in Druntime code, because it currently
 forces a lot of code that should be `nothrow` to not be annotated
 as such, even when the synchronization objects used use the
 standard D monitor implementation, which is actually `nothrow`.

 So I guess the way forward here for the Druntime code is to
 abandon the synchronized() statement and use locks directly?
Well, if polymorphism is involved at all, we're going to be screwed one way or another with regards to attributes. Either we're going to be very restrictive about what can be done in those functions by marking them with attributes like nogc and nothrow, or we're giong to be restrictive about what code can use those functions by not marking them with attributes like nogc or nothrow. This is part of why inheritance and polymorphism tend to be a very bad idea in D. OOP can obviously be very useful, but it comes at a definite cost. The only way to fix it so that we don't force one set of attributes or another would be to stop using polymorphism, and make it so that all of the code involved can be examined at compile time. So, either we figure out how to do that, or we have to pick which attributes we want and deal with the restrictions that come with them. That being said, I have a hard time believing that it makes sense for these primitives to involve exceptions - and even the GC likely shouldn't be involved, particularly since these aren't the sorts of operations that would normally allocate. So, I would think that aside from the breakage it would cause, marking them as nogc and nothrow would be perfectly acceptable. The problem is that changing the attributes on a class or interface tends to result in immediate code breakage, and I'm not sure that we really have a fix for that. - Jonathan M Davis
May 10 2016
prev sibling next sibling parent Mathias Lang <mathias.lang sociomantic.com> writes:
On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev wrote:
 Here is the situation, AIUI:

 1. We allow users to provide their own Monitors, which hook 
 into the synchronized(obj) statement.

 2. These monitors' methods are unadorned (no nothrow/ nogc).

 3. As a result, synchronized(obj) statements will not compile 
 in nothrow/ nogc code, because we can't know that the Monitor 
 implementation doesn't throw / use the GC.

 4. As Matthias mentioned, fixing this was attempted before (by 
 making the Monitor methods `nothrow`). However, this broke code 
 (vibe.d specifically), as its implementation was actually not 
 `nothrow`: https://issues.dlang.org/show_bug.cgi?id=11216

 This is a problem e.g. in Druntime code, because it currently 
 forces a lot of code that should be `nothrow` to not be 
 annotated as such, even when the synchronization objects used 
 use the standard D monitor implementation, which is actually 
 `nothrow`.

 So I guess the way forward here for the Druntime code is to 
 abandon the synchronized() statement and use locks directly?
As far as I remember, the issue was not the design, but that it required a design change on Vibe.d's side. See https://github.com/rejectedsoftware/vibe.d/pull/972#issuecomment-75714736 where Sonke mentions that "actually mutexes in vibe.d are never nothrow". However, since some of the internals of Vibe.d are now supported upstream, the problem might disappear in a foreseeable future.
May 10 2016
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
Why does vibe.d throw?

Also, any throwing code can be converted to nothrow with:

    try
    {
	...throwing code...
    }
    catch (Exception e)
    {
	...
    }
May 10 2016
next sibling parent =?UTF-8?Q?S=c3=b6nke_Ludwig?= <sludwig outerproduct.org> writes:
Am 11.05.2016 um 03:08 schrieb Walter Bright:
 Why does vibe.d throw?

 Also, any throwing code can be converted to nothrow with:

     try
     {
      ...throwing code...
     }
     catch (Exception e)
     {
      ...
     }
There are two kinds of fiber-aware mutexes in vibe.d, one that doesn't throw (created when this issue first came up) and the original one that can be interrupted from a different task using `Task.interrupt()`. In case of interruption it will throw an InterruptException instead of continuing to wait for the lock.
May 10 2016
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2016-05-11 03:08, Walter Bright wrote:
 Why does vibe.d throw?

 Also, any throwing code can be converted to nothrow with:

     try
     {
      ...throwing code...
     }
     catch (Exception e)
     {
      ...
     }
There's code in DWT and Tango as well that throws a SyncException exception when failing to acquire a lock. The code in DWT is basically copy pasted from Tango. Since druntime is derived from Tango I guess we can update the code in DWT and Tango to do what druntime does now, which is to throw an error instead. It's still a breaking change. Although I see now that there is a problem with updating the code in DWT. In some cases it catches SyncException with some logic based on that. In one case the try-catch is wrapped in a loop and will continue to try to acquire the look [1]. In another case it catches SyncException and triggers an interrupt [2], which I see now is not implemented [3]. Please advise how these two cases should be implemented when a lock will throw an error instead of exception (since errors should not be caught). Hmm, maybe I can extend the API. Monitor is not used directly in any of the cases. [1] https://github.com/d-widget-toolkit/org.eclipse.swt.gtk.linux.x86/blob/master/src/org/eclipse/swt/internal/Lock.d#L55 [2] https://github.com/d-widget-toolkit/org.eclipse.swt.gtk.linux.x86/blob/master/src/org/eclipse/swt/widgets/Synchronizer.d#L192-L198 [3] https://github.com/d-widget-toolkit/org.eclipse.swt.gtk.linux.x86/blob/master/src/org/eclipse/swt/internal/Compatibility.d#L402-L405 -- /Jacob Carlborg
May 11 2016
next sibling parent reply ZombineDev <petar.p.kirov gmail.com> writes:
On Wednesday, 11 May 2016 at 07:01:43 UTC, Jacob Carlborg wrote:
 On 2016-05-11 03:08, Walter Bright wrote:
     [...]
There's code in DWT and Tango as well that throws a SyncException exception when failing to acquire a lock. [...]
These cases all look like they want to use tryLock instead (http://dlang.org/phobos/core_sync_mutex#.Mutex.tryLock).
May 11 2016
parent Jacob Carlborg <doob me.com> writes:
On 2016-05-11 09:22, ZombineDev wrote:

 These cases all look like they want to use tryLock instead
 (http://dlang.org/phobos/core_sync_mutex#.Mutex.tryLock).
Ah, thanks. Although I see now that the code is not using Monitor.lock, it uses Condition.wait. Condition in this case is not the one in druntime, so I might be good. BTW, shouldn't there be a tryWait as well? -- /Jacob Carlborg
May 11 2016
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 5/11/2016 12:01 AM, Jacob Carlborg wrote:
 There's code in DWT and Tango as well that throws a SyncException exception
when
 failing to acquire a lock.

 The code in DWT is basically copy pasted from Tango. Since druntime is derived
 from Tango I guess we can update the code in DWT and Tango to do what druntime
 does now, which is to throw an error instead.

 It's still a breaking change.

 Although I see now that there is a problem with updating the code in DWT. In
 some cases it catches SyncException with some logic based on that. In one case
 the try-catch is wrapped in a loop and will continue to try to acquire the look
 [1].
Perhaps instead of throwing an exception it can try to re-acquire at the lower level.
 In another case it catches SyncException and triggers an interrupt [2],
 which I see now is not implemented [3].
I suppose the code has to decide if it is a fatal error or not.
May 11 2016
parent Jacob Carlborg <doob me.com> writes:
On 2016-05-11 13:10, Walter Bright wrote:

 Perhaps instead of throwing an exception it can try to re-acquire at the
 lower level.
There is basically no lower level, except the low level abstraction which wraps the Posix or Windows call. But it might be that the code is not affected. It's actually calling Condition.wait and not Monitor.lock in the cases where it catches the exception. -- /Jacob Carlborg
May 12 2016
prev sibling parent reply Dicebot <public dicebot.lv> writes:
On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev wrote:
 So I guess the way forward here for the Druntime code is to 
 abandon the synchronized() statement and use locks directly?
I believe this is the way. Synchronized statements don't add any crucial value compared to plain locks. At the same time forbidding throwing from even more runtime overrides would be both annoying and unnecessary restrictive.
May 11 2016
next sibling parent Guillaume Piolat <first.last gmail.com> writes:
On Wednesday, 11 May 2016 at 07:05:07 UTC, Dicebot wrote:
 On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev 
 wrote:
 So I guess the way forward here for the Druntime code is to 
 abandon the synchronized() statement and use locks directly?
I believe this is the way. Synchronized statements don't add any crucial value compared to plain locks. At the same time forbidding throwing from even more runtime overrides would be both annoying and unnecessary restrictive.
+1 For the sake of nogc, I can't use druntime locks or synchronized anyway.
May 11 2016
prev sibling next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 5/11/2016 12:05 AM, Dicebot wrote:
 I believe this is the way. Synchronized statements don't add any crucial value
 compared to plain locks. At the same time forbidding throwing from even more
 runtime overrides would be both annoying and unnecessary restrictive.
Also, the synchronized statement preceded the introduction of RAII, so perhaps it is obsolete.
May 11 2016
prev sibling parent reply Dicebot <public dicebot.lv> writes:
On Wednesday, 11 May 2016 at 07:05:07 UTC, Dicebot wrote:
 On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev 
 wrote:
 So I guess the way forward here for the Druntime code is to 
 abandon the synchronized() statement and use locks directly?
I believe this is the way. Synchronized statements don't add any crucial value compared to plain locks. At the same time forbidding throwing from even more runtime overrides would be both annoying and unnecessary restrictive.
It is probably also worth re-iterating on my long standing position that adding more `nothrow` in fundamental facilities is a false goal and almost always does more harm than good. Exceptions are main (and pretty much only) error handling facility in D. By adding `nothrow` base runtime class methods you make impossible for D developers to use standard language facilities and force uncalled hacks to workaround it. `nothrow` is much more intrusive than ` safe` or `pure` in that sense and should not be applied blindly to everything simply because it is possible.
May 11 2016
next sibling parent Vladimir Panteleev <thecybershadow.lists gmail.com> writes:
On Wednesday, 11 May 2016 at 11:16:41 UTC, Dicebot wrote:
 It is probably also worth re-iterating on my long standing 
 position that adding more `nothrow` in fundamental facilities 
 is a false goal and almost always does more harm than good. 
 Exceptions are main (and pretty much only) error handling 
 facility in D. By adding `nothrow` base runtime class methods 
 you make impossible for D developers to use standard language 
 facilities and force uncalled hacks to workaround it.

 `nothrow` is much more intrusive than ` safe` or `pure` in that 
 sense and should not be applied blindly to everything simply 
 because it is possible.
Sometimes you really, actually must ensure that things aren't going to throw, such as in a signal handler, unmanaged thread, between fork and exec, bare-metal, ... Your point is orthogonal to the goal of adding nothrow/ nogc without breaking user code, and I don't disagree with it.
May 11 2016
prev sibling parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Wednesday, May 11, 2016 11:16:41 Dicebot via Digitalmars-d wrote:
 On Wednesday, 11 May 2016 at 07:05:07 UTC, Dicebot wrote:
 On Tuesday, 10 May 2016 at 17:46:17 UTC, Vladimir Panteleev

 wrote:
 So I guess the way forward here for the Druntime code is to
 abandon the synchronized() statement and use locks directly?
I believe this is the way. Synchronized statements don't add any crucial value compared to plain locks. At the same time forbidding throwing from even more runtime overrides would be both annoying and unnecessary restrictive.
It is probably also worth re-iterating on my long standing position that adding more `nothrow` in fundamental facilities is a false goal and almost always does more harm than good. Exceptions are main (and pretty much only) error handling facility in D. By adding `nothrow` base runtime class methods you make impossible for D developers to use standard language facilities and force uncalled hacks to workaround it. `nothrow` is much more intrusive than ` safe` or `pure` in that sense and should not be applied blindly to everything simply because it is possible.
Regardless of the desirability of marking stuff with nothrow, one _huge_ difference between nothrow and pure or nogc is that it's trivial to use a function that throws inside of nothrow code by wrapping it in a try-catch block. safe is in a similar boat thanks to trusted, but pure and nogc can only be gotten around via some nasty casts. That being said, I think that anything that really shouldn't ever throw should be marked nothrow, but we need to be sure that it really doesn't ever make sense for it to throw before we mark it as nothrow. If it does make sense for it to throw under any circumstances, then we can't mark it as nothrow, and then anyone who wants nothrow is going to have to use try-catch blocks to use it in nothrow code. - Jonathan M Davis
May 11 2016
parent reply Dicebot <public dicebot.lv> writes:
On Wednesday, 11 May 2016 at 14:12:47 UTC, Jonathan M Davis wrote:
 Regardless of the desirability of marking stuff with nothrow, 
 one _huge_ difference between nothrow and pure or  nogc is that 
 it's trivial to use a function that throws inside of nothrow 
 code by wrapping it in a try-catch block.  safe is in a similar 
 boat thanks to  trusted, but pure and  nogc can only be gotten 
 around via some nasty casts.
It doesn't apply if throwing is desired as part of API, which is exactly the case for monitor and issue I had before with adding nothrow to some Fiber methods.
May 11 2016
parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Wednesday, May 11, 2016 14:22:39 Dicebot via Digitalmars-d wrote:
 On Wednesday, 11 May 2016 at 14:12:47 UTC, Jonathan M Davis wrote:
 Regardless of the desirability of marking stuff with nothrow,
 one _huge_ difference between nothrow and pure or  nogc is that
 it's trivial to use a function that throws inside of nothrow
 code by wrapping it in a try-catch block.  safe is in a similar
 boat thanks to  trusted, but pure and  nogc can only be gotten
 around via some nasty casts.
It doesn't apply if throwing is desired as part of API, which is exactly the case for monitor and issue I had before with adding nothrow to some Fiber methods.
I wasn't disagreeing with that. If there are valid uses cases for a monitor throwing, then its functions should not be nothrow. My point is that if those functions aren't marked with nothrow, it's trivial to use them in nothrow code (even if it's a bit ugly) by wrapping them in try-catch blocks, whereas attributes like pure or nogc are not so easy to get around. pure wouldn't apply in this case, but nogc might. However, until the issues with using exceptions without the GC are resolved, as long as monitor's functions can be nothrow, it makes no sense to mark them with nogc. So, the lack of nothrow is far from a blocker for anyone wanting nothrow code. It's nogc that's more of an issue, but we're going to need language improvements before we can consider using nogc in this case. - Jonathan M Davis
May 11 2016
parent Dicebot <public dicebot.lv> writes:
On Wednesday, 11 May 2016 at 14:54:07 UTC, Jonathan M Davis wrote:
 On Wednesday, May 11, 2016 14:22:39 Dicebot via Digitalmars-d 
 wrote:
 On Wednesday, 11 May 2016 at 14:12:47 UTC, Jonathan M Davis 
 wrote:
 Regardless of the desirability of marking stuff with 
 nothrow, one _huge_ difference between nothrow and pure or 
  nogc is that it's trivial to use a function that throws 
 inside of nothrow code by wrapping it in a try-catch block. 
  safe is in a similar boat thanks to  trusted, but pure and 
  nogc can only be gotten around via some nasty casts.
It doesn't apply if throwing is desired as part of API, which is exactly the case for monitor and issue I had before with adding nothrow to some Fiber methods.
I wasn't disagreeing with that. If there are valid uses cases for a monitor throwing, then its functions should not be nothrow. My point is that if those functions aren't marked with nothrow, it's trivial to use them in nothrow code (even if it's a bit ugly) by wrapping them in try-catch blocks, whereas attributes like pure or nogc are not so easy to get around. pure wouldn't apply in this case, but nogc might. However, until the issues with using exceptions without the GC are resolved, as long as monitor's functions can be nothrow, it makes no sense to mark them with nogc. So, the lack of nothrow is far from a blocker for anyone wanting nothrow code. It's nogc that's more of an issue, but we're going to need language improvements before we can consider using nogc in this case. - Jonathan M Davis
Sorry, I got your point exactly backwards :)
May 11 2016