Skip to content

change deleteDirectoryTask to a List #321

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

Merged
merged 2 commits into from
Sep 13, 2015
Merged

change deleteDirectoryTask to a List #321

merged 2 commits into from
Sep 13, 2015

Conversation

joshhe
Copy link

@joshhe joshhe commented Sep 8, 2015

I am aware if the jvm exists, it will remove all created temp folders. However, I need to keep the jvm running as long as possible in production, which eventually cause the temp folder to fill up with tons of temp folders created by the proxy.

Fix: changed DeleteDirectoryTask deleteDirectoryTask to List deleteDirectoryTasks in SeleniumProxyHandler class, so that when user need to clean up the temp folders but keep the jvm running they can just call proxy.cleanSslCertificates() method.

@@ -76,7 +72,7 @@
private final boolean proxyInjectionMode;
private final boolean forceProxyChain;
private boolean fakeCertsGenerated;
private DeleteDirectoryTask deleteDirectoryTask;
private List<DeleteDirectoryTask> deleteDirectoryTasks = new ArrayList<DeleteDirectoryTask>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrap this in Collections.synchronizedList() and make this final.

@jekh
Copy link
Collaborator

jekh commented Sep 8, 2015

This looks good to me, and I'm surprised we didn't have this before. I made a few comments to make it thread safe.

@joshhe
Copy link
Author

joshhe commented Sep 9, 2015

Thanks for the comment. I update the fix as your commented out. Let me know if I need to make more changes.

@joshhe
Copy link
Author

joshhe commented Sep 9, 2015

Now I am seeing this exception:

java.io.FileNotFoundException: /tmp/seleniumSslSupportwww.google-analytics.com4561906671811516371/keymap.ser (No such file or directory)
at java.io.FileOutputStream.open0(Native Method)
at java.io.FileOutputStream.open(FileOutputStream.java:270)
at java.io.FileOutputStream.(FileOutputStream.java:213)
at java.io.FileOutputStream.(FileOutputStream.java:162)
at net.lightbody.bmp.proxy.selenium.KeyStoreManager.persistKeyPairMap(KeyStoreManager.java:677)
at net.lightbody.bmp.proxy.selenium.KeyStoreManager.rememberKeyPair(KeyStoreManager.java:694)
at net.lightbody.bmp.proxy.selenium.KeyStoreManager.getRSAKeyPair(KeyStoreManager.java:642)
at net.lightbody.bmp.proxy.selenium.KeyStoreManager.getMappedCertificateForHostname(KeyStoreManager.java:550)
at net.lightbody.bmp.proxy.selenium.KeyStoreManager.getCertificateByHostname(KeyStoreManager.java:389)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.wireUpSslWithCyberVilliansCA(SeleniumProxyHandler.java:624)
at net.lightbody.bmp.proxy.BrowserMobProxyHandler.wireUpSslWithCyberVilliansCA(BrowserMobProxyHandler.java:112)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.getSslRelayOrCreateNew(SeleniumProxyHandler.java:544)
at net.lightbody.bmp.proxy.BrowserMobProxyHandler.getSslRelayOrCreateNew(BrowserMobProxyHandler.java:117)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.handleConnect(SeleniumProxyHandler.java:492)
at net.lightbody.bmp.proxy.BrowserMobProxyHandler.handleConnect(BrowserMobProxyHandler.java:96)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.handle(SeleniumProxyHandler.java:184)
at net.lightbody.bmp.proxy.jetty.http.HttpContext.handle(HttpContext.java:1509)
at net.lightbody.bmp.proxy.jetty.http.HttpContext.handle(HttpContext.java:1461)
at net.lightbody.bmp.proxy.jetty.http.HttpServer.service(HttpServer.java:892)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.service(HttpConnection.java:815)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.handleNext(HttpConnection.java:981)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.handle(HttpConnection.java:832)
at net.lightbody.bmp.proxy.jetty.http.SocketListener.handleConnection(SocketListener.java:245)
at net.lightbody.bmp.proxy.jetty.util.ThreadedServer.handle(ThreadedServer.java:357)
at net.lightbody.bmp.proxy.jetty.util.ThreadPool$PoolThread.run(ThreadPool.java:534)

@jekh
Copy link
Collaborator

jekh commented Sep 9, 2015

Okay, I'll hold off on merging this until that error is resolved.

@joshhe
Copy link
Author

joshhe commented Sep 9, 2015

I changed back to beta2, I see this exception in the log:

java.lang.RuntimeException: Can't start SslRelay: server is not started (perhaps it was just shut down?)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.getSslRelayOrCreateNew(SeleniumProxyHandler.java:563)
at net.lightbody.bmp.proxy.BrowserMobProxyHandler.getSslRelayOrCreateNew(BrowserMobProxyHandler.java:117)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.handleConnect(SeleniumProxyHandler.java:496)
at net.lightbody.bmp.proxy.BrowserMobProxyHandler.handleConnect(BrowserMobProxyHandler.java:96)
at net.lightbody.bmp.proxy.selenium.SeleniumProxyHandler.handle(SeleniumProxyHandler.java:188)
at net.lightbody.bmp.proxy.jetty.http.HttpContext.handle(HttpContext.java:1509)
at net.lightbody.bmp.proxy.jetty.http.HttpContext.handle(HttpContext.java:1461)
at net.lightbody.bmp.proxy.jetty.http.HttpServer.service(HttpServer.java:892)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.service(HttpConnection.java:815)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.handleNext(HttpConnection.java:981)
at net.lightbody.bmp.proxy.jetty.http.HttpConnection.handle(HttpConnection.java:832)
at net.lightbody.bmp.proxy.jetty.http.SocketListener.handleConnection(SocketListener.java:245)
at net.lightbody.bmp.proxy.jetty.util.ThreadedServer.handle(ThreadedServer.java:357)
at net.lightbody.bmp.proxy.jetty.util.ThreadPool$PoolThread.run(ThreadPool.java:534)

Do you know what's possible cause for it?

@jekh
Copy link
Collaborator

jekh commented Sep 9, 2015

Are you saying you see this issue when you apply the change in this PR to beta-2?

@joshhe
Copy link
Author

joshhe commented Sep 10, 2015

No, I see the above error when I am running beta2 without my changes.

@jekh
Copy link
Collaborator

jekh commented Sep 10, 2015

If it's not related to this PR, you should probably open a separate issue for it. What are the results of your tests with this PR build?

@jekh
Copy link
Collaborator

jekh commented Sep 13, 2015

This looks good to me, so I'm inclined to merge it before releasing beta 3. @chemicwepn - if you do see any issues with this change, please feel free to submit another PR.

jekh added a commit that referenced this pull request Sep 13, 2015
change deleteDirectoryTask to a List
@jekh jekh merged commit ed2454a into lightbody:master Sep 13, 2015
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