Skip to content

Conversation

@pierreluctg
Copy link
Collaborator

Fixing memory leak in neoVI bus where message_receipts grows with no limit. message_receipts was growing without bound on msg Rx side

@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Merging #1427 (a89ca23) into develop (cd51ec4) will increase coverage by 0.02%.
The diff coverage is 0.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1427      +/-   ##
===========================================
+ Coverage    65.26%   65.28%   +0.02%     
===========================================
  Files           81       81              
  Lines         8827     8830       +3     
===========================================
+ Hits          5761     5765       +4     
+ Misses        3066     3065       -1     

message_receipts was growing without bound on msg Rx side
@pierreluctg pierreluctg marked this pull request as ready for review November 8, 2022 15:42
@pierreluctg pierreluctg added this to the 4.0.1 milestone Nov 8, 2022
@pierreluctg pierreluctg requested a review from felixdivo November 8, 2022 15:43
@felixdivo
Copy link
Collaborator

We should add some typing information for constructs like self.message_receipts to make it easier to read what's in these generic containers. 😄

Copy link
Collaborator

@felixdivo felixdivo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@pierreluctg pierreluctg merged commit 5f485db into develop Nov 9, 2022
@pierreluctg pierreluctg deleted the neovi-nem-leak branch November 9, 2022 17:08
@pierreluctg
Copy link
Collaborator Author

@hardbyte and @felixdivo do we have ETA or some kind of goal on when we want to release 4.0.1?

@felixdivo
Copy link
Collaborator

At least for 4.0.0 we did not have a real time estimate. I just sat down at some point and proposed a date (and missed it ^^).

I'm a fan of releasing more often than previously, since I personally find it very annoying if you contribute a bugfix to a repo and they do not release it for ages. So you are forced to install from source or stuff like that.

So now would be a good time for the next release, but since I will not prepare it I don't want to push hard. Maybe include #1385 since it seems to be already finished.

BTW: There is #1363 by @zariiii9003 for 4.1.0, which might be a better place to discuss it than here.

@pierreluctg pierreluctg modified the milestones: 4.0.1, 4.1.0 Nov 10, 2022
@pierreluctg
Copy link
Collaborator Author

@felixdivo there will be no 4.0.1 release? If this is the case can we close the milestone to avoid confusion?

@felixdivo
Copy link
Collaborator

It certainly did confuse me. If there will be no such release, we should reassign all existing (also closed & merged!) issues/PRs to 4.1.0. @zariiii9003 should I do that? I think you have a good overview over what makes sense now.

@zariiii9003
Copy link
Collaborator

We agreed on changing it to 4.1.0 because of the can.logger API changes. If you don't mind updating those PRs, that would be cool :)

Once #1385 and #1363 are merged, we should create a new release.

@felixdivo
Copy link
Collaborator

Sounds good 👍 will do, latest Saturday

@j-c-cook
Copy link
Contributor

#1385 is migrated to #1429.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants