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

Fix #8844 Avoid hardcoded #doTerminationFromYourself as a criteria #8845

Conversation

isCzech
Copy link
Contributor

@isCzech isCzech commented Mar 21, 2021

Fixes #8844

@MarcusDenker MarcusDenker added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Mar 23, 2021
@MarcusDenker
Copy link
Member

Somehow it looks as if the tests time out on all platforms

@isCzech
Copy link
Contributor Author

isCzech commented Mar 23, 2021

Somehow it looks as if the tests time out on all platforms

Thanks, it looks like some debugger tests don't like the feature preventing 'cannot return' errors (i.e. simply suspending accidentally resumed terminated processes, without raising an error). So I just commented the feature out (leaving the other code intact) to make it available for future consideration. Sorry for missing this :)

@dionisiydk
Copy link
Contributor

Hi @isCzech .

Sorry that I was not clear. But I expected that you will push your changes on top of mine.

Now it will cause a conflict with my PR (#8567 ). And considering the size of my changes I would be very happy to avoid remerging. It will require another pass on testing and rethinking of what I get.
Not sure that I will have the time for this soon. But the PR fixes very critical problems. it is better to integrate it first and then do other improvements.

So please wait the integration of #8567

@isCzech
Copy link
Contributor Author

isCzech commented Mar 23, 2021

Hi @isCzech .

Sorry that I was not clear. But I expected that you will push your changes on top of mine.

So please wait the integration of #8567

Hi @dionisiydk ,
sure! I hope the proposed changes will still be compatible :)

@Ducasse
Copy link
Member

Ducasse commented Mar 23, 2021

Hi denis

I heard today Pablo telling that he had a look and like it but wants guille to have a look too so that they can make sure that it is ok.

S.

@isCzech
Copy link
Contributor Author

isCzech commented Apr 8, 2021

Hi All,
I've opened an issue #8992 (Unwind mechanism during termination is broken - skips multiple unwind blocks and corrupts the image) which addresses a related but more important problem. Adjusting #isTerminated suggested here is of a way lower priority.

@Ducasse Ducasse added the Priority: Critical To fix or review as soon as possible label Jun 3, 2021
@estebanlm
Copy link
Member

same as with #8993, we want this, but is very dangerous so I am deferring.

@Ducasse
Copy link
Member

Ducasse commented Jun 18, 2021

Yes this is good to relabel it.

@guillep
Copy link
Member

guillep commented Nov 24, 2021

Hi all, it's been really difficult to review the batch of issues (#8567 , #8845, #9137, #8993, #8509).
We have spent in the last internal sprints several hours taking a look at them, but it is a reality that those changes are difficult to review, some like #8567 are very big, their effects in the system are difficult to foresee independently and even worse in combination.

@tesonep is correctly (!!) proposing to my ear that we make a single PR joining all of them to reduce the merge noise, let the CI test the combination of all issues. I then propose that we eagerly merge this (meta) PR and be ready to quickly apply hot-fixes as soon as possible or revert it in case of fire.

Thoughts? @dionisiydk @Ducasse @isCzech @estebanlm

@estebanlm
Copy link
Member

Ok for me :)

@guillep
Copy link
Member

guillep commented Nov 24, 2021

Superseeded by #10429

@guillep guillep closed this Nov 24, 2021
@Ducasse
Copy link
Member

Ducasse commented Nov 24, 2021

Agreed because after they become baby monsters that grow up and that we cannot handle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical To fix or review as soon as possible Status: Need more work The issue is nearly ready. Waiting some last bits.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid hardcoded #doTerminationFromYourself as a criteria for the isTerminated method
6 participants