Skip to content

Prevent planner from locking up on large problems. #8

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luke-clifton
Copy link

In some scenarios, the downward executable was producing a lot of
output which was not being read by the library. This would eventually
cause the downward process to hang, waiting for someone to consume its
output. This change drains both output handles before calling wait, and
thus preventing the deadlock.

In some scenarios, the `downward` executable was producing a lot of
output which was not being read by the library. This would eventually
cause the downward process to hang, waiting for someone to consume its
output. This change drains both output handles before calling wait, and
thus preventing the deadlock.
@luke-clifton
Copy link
Author

Other solution would be to not pipe the outputs seeing as they aren't being used.

@ocharles
Copy link
Contributor

This is interesting, I'm not sure I've observed this at all in years of running this in production. Do you have any way to reproduce this?

@luke-clifton
Copy link
Author

Set the list length as required to make downward spit out more than whatever your OS is allowing it to buffer. This patch will make it work.

import FastDownward                                           
import FastDownward.Exec as Exec                              
                                                              
data Add = Add Int                                            
                                                              
main :: IO ()                                                 
main = do                                                     
    let                                                       
        initial :: [Int]                                      
        initial = [0..500]                                    
    res <- runProblem $ do                                    
        vars <- mapM newVar initial                           
        solve Exec.bjolp                                      
            (map (\i -> modifyVar (vars !! i) negate) initial)
            (zipWith (?=) vars (map negate initial))          
    print res                                                                                                          

@ocharles
Copy link
Contributor

Thanks!

@luke-clifton
Copy link
Author

You can tell it's hung by this (and not because the problem is too large) because your CPU will not be busy, as all processes are sleeping, downward waiting for your app to read from the pipes, and your app is waiting for downward to finish.

@luke-clifton
Copy link
Author

Did you manage to reproduce? 90 was enough to overflow the buffers on macOS and on my NixOS machine. That is, 90 vars, with 90 effects.

I've stuck the complete example in a gist.

https://gist.github.com/luke-clifton/29df0f6cc0664a3eaa64e6e433506cc5

Technically, one should really be reading from both of those handles at the same time. If the stderr handle fills up, and we are still trying to read the stdout handle, it would block as well, a fact that can be witnessed by swapping the order that we read the inputs in in this patch and seeing that it will lock up again.

To combat that we could possible use Control.Concurrent.Async.concurrently

  (stderr, stdout) <- concurrently
        (Data.Text.IO.hGetContents stderrHandle)
        (Data.Text.IO.hGetContents stdoutHandle)

Which would be totally safe at the cost of adding async as a direct dependency. (It is already a transitive one though).

@ocharles
Copy link
Contributor

I'm afraid I haven't had a chance to look yet. I hope to look soon.

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.

2 participants