Fix compilation on DMD v2.067 #992

etcimon · 2015-02-25T17:03:28Z

This also adds a fix for HashMap reallocation issue pointed out earlier.

Geod24 · 2015-02-25T18:14:21Z

source/vibe/core/drivers/libasync.d

@@ -760,7 +761,7 @@ final class LibasyncManualEvent : ManualEvent {
 				auto evloop = getEventLoop();
 				shared AsyncSignal sig = cast(shared AsyncSignal) signal;
 				if (!sig.trigger(evloop))
-					throw new Exception(sig.error);
+					return; //throw new Exception(sig.error);


In which situation could this happen ? Wouldn't an asset be better over a silent fail ?

Oops, that's an artifact when I was experimenting making this method nothrow

s-ludwig · 2015-02-26T06:45:00Z

Still fails, because libasync.all doesn't exist.

etcimon · 2015-02-26T12:22:40Z

Still fails, because libasync.all doesn't exist.

Yes, that one was removed recently in favor of just libasync

Geod24 · 2015-02-26T14:25:49Z

It's in need for a rebase.

I still don't consider 7cca2ba as a good solution though, especially given the fact it turns an Exception into an Error.

etcimon · 2015-02-26T14:47:00Z

I think the proper solution is to stop inheriting from Object.Monitor, stop using synchronized in fibers, and write a scopedLock!TaskMutex for it instead (so that fibers can be interrupted!)

mihails-strasuns · 2015-02-26T14:47:16Z

turns an Exception into an Error

Into segmentation fault actually (with -release) ;)

etcimon · 2015-02-26T14:49:15Z

Into segmentation fault actually (with -release) ;)

I got a strange segmentation fault in -release using dmd master with libasync (Windows). This isn't supposed to happen, what platform are you on.

etcimon · 2015-02-26T15:11:10Z

Nevermind, that's actually what usually happens on assertion failures in release ^^

etcimon · 2015-02-26T16:03:16Z

I hope this commit will settle the question :)

s-ludwig · 2015-02-26T18:21:51Z

Sorry, but in this form this is not a solution. This is all public code and changing the inheritance like this is a breaking change. All breaking changes must at least go through a deprecation phase.

etcimon · 2015-02-26T18:40:06Z

All breaking changes must at least go through a deprecation phase.

How do you deprecate broken code (on DMD 2.067)? Unless I wrap it in a static if version clause?

etcimon · 2015-02-26T20:17:34Z

This new fix adds some features when running DMD <= 2.066, and removes other features for DMD 2.067 to make vibe.d compatible with it.

Unfortunately, it'll break anything that tries to run on DMD 2.067 with certain mutex uses, but that was already broken and is simply bubbling up from druntime.

I don't know how to deprecate it properly, but the changes for DMD >= 2.067 are as follows:

TaskMutex, RecursiveTaskMutex, TaskCondition are now detached from Object.Monitor on the inheritance tree
synchronized statement is unavailable as a consequence, it must be replaced with auto l = mutex.locked, or auto l = scopedLock(mutex) which returns a ScopedMutexLock
If using DMD >= 2.067, the with statement is also available as a convenience (it doesn't work on <= 2.066): with(mutex.locked) { /* synchronized */ }

s-ludwig · 2015-02-26T20:57:23Z

What would have to be deprecated is the old API. This would for example be the whole TaskMutex class. A new class would then have to be added as a replacement. But maybe there is a more elegant path, too... maybe using some alias this trickery. But the final direction taken in Druntime needs to be clear first.

BTW, TaskMutex.locked is redundant and doesn't save much over m.scopedLock (UFCS). I'd remove it to avoid confusion.

etcimon · 2015-02-26T21:19:53Z

Didn't think the UFCS would allow me to remove parenthesis.

So this last fix should be good, if you can merge the travis exclusion I'm sure we can see the whole thing go green.

s-ludwig · 2015-02-26T21:35:09Z

I would have liked to keep the mutex changes separate from the libasync ones (defer it until the Druntime case is settled), but anyway, that can always be changed again before release. But the first commit still seems to contain merge errors (reverts some changes in the libevent driver), or was that intended (Travis still passes)?

etcimon · 2015-02-26T21:38:54Z

But the first commit still seems to contain merge errors (reverts some changes in the libevent driver)

That was intentional, the changes I submitted are in a different direction and the nothrow / static assert(false) are unnecessary now. This fix doesn't affect the DMD <= 2.066.1 builds

I would have liked to keep the mutex changes separate from the libasync ones (defer it until the Druntime case is settled)

They allow libasync to run on 2.067, which is relevant to the pull :)

etcimon · 2015-02-26T21:42:08Z

(Travis still passes)?

Travis passes on everything except the 4 versions that aren't supported by libasync (yet)

s-ludwig · 2015-02-26T21:47:31Z

So did you make the changes in libevent.d on purpose? Why are they in the commit that fixes the libasync driver?

etcimon · 2015-02-26T21:54:47Z

So did you make the changes in libevent.d on purpose? Why are they in the commit that fixes the libasync driver?

They're in the libasync driver pull because it must address the whole of the mutex issues in 2.067, where the proposed solution was to make the drivers nothrow. The libevent driver had a few added nothrows, which I removed because it seemed unnecessary given the fact that the new approach is to let the mutexes and drivers remain throwable.

I can revert the changes if you like.

s-ludwig · 2015-02-26T21:58:27Z

I can revert the changes if you like.

No no, I think it's most probably a step in the right direction. It just seemed out of place in the commit order back then.

Made libevent default again Fix dub.json Remove artifact Add assertions to replace caught exceptions Update version for travis tests Remove libasync.all Add sorting import for redis test Implement throwable task mutexes Removed memory changes fix segfault Lock using with statement in synchronized-like syntax Add compatibility with previous DMD versions Add deprecation messages for TaskMutex & RecursiveTaskMutex update test

etcimon · 2015-02-27T18:45:22Z

Green!

s-ludwig · 2015-02-27T19:19:14Z

Yay!

Fix compilation on DMD v2.067

s-ludwig · 2015-02-27T19:19:36Z

And thanks of course!

MartinNowak · 2015-03-04T04:45:42Z

source/vibe/core/sync.d

+	@trusted bool tryLock();
+	@trusted void lock();
+	@trusted void unlock();
+}


This won't work with synchronized and really looks like an abysmal solution.

etcimon mentioned this pull request Feb 25, 2015

libasync configuration compilation errors #988

Closed

Geod24 reviewed Feb 25, 2015
View reviewed changes

s-ludwig added a commit that referenced this pull request Feb 27, 2015

Merge pull request #992 from etcimon/fix-libasync-new-dmd

c3e0a8c

Fix compilation on DMD v2.067

s-ludwig merged commit c3e0a8c into vibe-d:master Feb 27, 2015

etcimon deleted the fix-libasync-new-dmd branch February 27, 2015 19:43

MartinNowak reviewed Mar 4, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix compilation on DMD v2.067 #992

Fix compilation on DMD v2.067 #992

etcimon commented Feb 25, 2015

Geod24 Feb 25, 2015

etcimon Feb 25, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

Geod24 commented Feb 26, 2015

etcimon commented Feb 26, 2015

mihails-strasuns commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 27, 2015

s-ludwig commented Feb 27, 2015

s-ludwig commented Feb 27, 2015

MartinNowak Mar 4, 2015

Fix compilation on DMD v2.067 #992

Fix compilation on DMD v2.067 #992

Conversation

etcimon commented Feb 25, 2015

Geod24 Feb 25, 2015

Choose a reason for hiding this comment

etcimon Feb 25, 2015

Choose a reason for hiding this comment

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

Geod24 commented Feb 26, 2015

etcimon commented Feb 26, 2015

mihails-strasuns commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 26, 2015

s-ludwig commented Feb 26, 2015

etcimon commented Feb 27, 2015

s-ludwig commented Feb 27, 2015

s-ludwig commented Feb 27, 2015

MartinNowak Mar 4, 2015

Choose a reason for hiding this comment