Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove refsLeft assert #128

Merged
merged 1 commit into from
Jan 23, 2019
Merged

Conversation

WebFreak001
Copy link
Contributor

It seems when running in a vibe-d app listening to HTTP requests that the assert fails. Even though I would like there to be a check, closing the vibe.d app afterwards shows no leaked handles so I think it's fine, and definitely better than not starting anymore at all.

It seems when running in a vibe-d app that the assert fails
Even though I would like there to be a check, closing the vibe.d
app afterwards shows no leaked handles so I think it's fine.
@s-ludwig s-ludwig merged commit 177e761 into vibe-d:master Jan 23, 2019
@s-ludwig
Copy link
Member

Would be nice to have a reproduction case for this. Is it enough to start a HTTP server and then attempt a failing outgoing connection?

@WebFreak001
Copy link
Contributor Author

yep this triggers it:

import vibe.vibe;

void main()
{
	runTask({
		auto conn = connectTCP("127.0.0.1", 16516);
		assert(false);
	});

	auto set = new HTTPServerSettings();
	set.port = 8080;
	listenHTTP(set, &index);

	runEventLoop();
}

void index(scope HTTPServerRequest req, scope HTTPServerResponse res)
{
}

@s-ludwig
Copy link
Member

Okay, thanks, it even triggers without the HTTP server for me: https://run.dlang.io/is/x97pL3

The reason is that within a task, the task will be resumed from within the eventcore callback, during which there is still an extra reference present to keep the FD valid.

On Windows it actually works, because it has different semantics (it only calls the callback if there is at least one reference left). I need to think about this some more before unifying them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants