This repository has been archived by the owner. It is now read-only.

IOWatcher broken since 0.7.9 #4136

Closed
agnat opened this Issue Oct 14, 2012 · 5 comments

Comments

Projects
None yet
2 participants
@agnat

agnat commented Oct 14, 2012

Since 0.7.9 node exits even if an IOWatcher is still active. IO works fine when I keep the process running by adding e.g. a timer. My best guess is that the reference counting is broken. Although IOWatcher is deprecated and removed in 0.9 I think it should be fixed in 0.7 and 0.8.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Oct 14, 2012

Member

My best guess is that the reference counting is broken.

That's correct.

Although IOWatcher is deprecated and removed in 0.9 I think it should be fixed in 0.7 and 0.8.

I agree but I don't think it's possible without changing the C++ ABI and that requires recompiling every module that directly or indirectly depends on the IOWatcher class.

I don't really want to do that but I'll leave the issue open. If enough people speak up I'll reconsider it.

Member

bnoordhuis commented Oct 14, 2012

My best guess is that the reference counting is broken.

That's correct.

Although IOWatcher is deprecated and removed in 0.9 I think it should be fixed in 0.7 and 0.8.

I agree but I don't think it's possible without changing the C++ ABI and that requires recompiling every module that directly or indirectly depends on the IOWatcher class.

I don't really want to do that but I'll leave the issue open. If enough people speak up I'll reconsider it.

@agnat

This comment has been minimized.

Show comment
Hide comment
@agnat

agnat Oct 15, 2012

Well, to me IOWatcher is a critical component. See #3348. Considering that I'm trying to convince you guys to replace it with something uv poll based I'd really like to see it fixed. The mdns addon is not working properly with the affected node versions. It just doesn't feel right to tell people I can't do anything about it "because it is an upstream bug".

agnat commented Oct 15, 2012

Well, to me IOWatcher is a critical component. See #3348. Considering that I'm trying to convince you guys to replace it with something uv poll based I'd really like to see it fixed. The mdns addon is not working properly with the affected node versions. It just doesn't feel right to tell people I can't do anything about it "because it is an upstream bug".

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Oct 15, 2012

Member

The mdns addon is not working properly with the affected node versions. It just doesn't feel right to tell people I can't do anything about it "because it is an upstream bug".

IOWatcher is gone completely in master so you'll have to make the switch to uv_poll_t eventually, might as well do it now.

Member

bnoordhuis commented Oct 15, 2012

The mdns addon is not working properly with the affected node versions. It just doesn't feel right to tell people I can't do anything about it "because it is an upstream bug".

IOWatcher is gone completely in master so you'll have to make the switch to uv_poll_t eventually, might as well do it now.

@agnat

This comment has been minimized.

Show comment
Hide comment
@agnat

agnat Oct 15, 2012

I still think the wrapper for uv_poll_t belongs on the node side. See my comment on #3348.

agnat commented Oct 15, 2012

I still think the wrapper for uv_poll_t belongs on the node side. See my comment on #3348.

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Oct 9, 2013

Member

Closing. v0.8 is in maintenance mode and IOWatcher is gone in v0.10.

Member

bnoordhuis commented Oct 9, 2013

Closing. v0.8 is in maintenance mode and IOWatcher is gone in v0.10.

@bnoordhuis bnoordhuis closed this Oct 9, 2013

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.