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

Upgraded address stops working after all references are dropped #38

Closed
fuchsnj opened this issue Jan 15, 2018 · 5 comments
Closed

Upgraded address stops working after all references are dropped #38

fuchsnj opened this issue Jan 15, 2018 · 5 comments
Assignees
Labels
C-bug Category: bug

Comments

@fuchsnj
Copy link
Contributor

fuchsnj commented Jan 15, 2018

If you upgrade an Address to a SyncAddress, then drop the SyncAddress, then upgrade the address again, the SyncAddress returned during the 2nd upgrade does not work.

I was able to create a minimal example showing this bug
https://github.com/fuchsnj/actix_bug/blob/master/src/main.rs

The example program upgrades an address twice, each time sending a message that should be printed. It should print "Handling message!" twice, but it's only printing once.

I believe this is because when the last reference to SyncAddress is dropped, the underlying sender is closed. If you upgrade an address after this, it re-uses the tx/rx from the first one that was already closed.

In my example code, you can uncomment one of the lines which prevents the SyncAddress from being dropped, and the bug goes away.

Also, if you comment out the line in actix that closes this sender, the bug also goes away.
https://github.com/actix/actix/blob/master/src/queue/sync.rs#L763

@fuchsnj fuchsnj added the C-bug Category: bug label Jan 15, 2018
@fafhrd91
Copy link
Member

Very nice!

Would you provide PR?

@fuchsnj
Copy link
Contributor Author

fuchsnj commented Jan 15, 2018

I'll open a PR for this one. Still need to get more familiar with the code to figure out the best way to solve this one. If you have any ideas that would be helpful.

@fuchsnj fuchsnj self-assigned this Jan 15, 2018
@fafhrd91
Copy link
Member

I am traveling at the moment, won’t be able to help next 10 hours

@fafhrd91
Copy link
Member

this is actually tricky problem, let me fix it

@fafhrd91
Copy link
Member

fixed in master

semtexzv pushed a commit that referenced this issue Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants