{"action":"created","issue":{"url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/89","labels_url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/89/labels{/name}","comments_url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/89/comments","events_url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/89/events","html_url":"https://github.com/DigitalPebble/storm-crawler/issues/89","id":56750964,"number":89,"title":"Concurrency issue with ProtocolFactory","user":{"login":"jnioche","id":218319,"avatar_url":"https://avatars.githubusercontent.com/u/218319?v=3","gravatar_id":"","url":"https://api.github.com/users/jnioche","html_url":"https://github.com/jnioche","followers_url":"https://api.github.com/users/jnioche/followers","following_url":"https://api.github.com/users/jnioche/following{/other_user}","gists_url":"https://api.github.com/users/jnioche/gists{/gist_id}","starred_url":"https://api.github.com/users/jnioche/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jnioche/subscriptions","organizations_url":"https://api.github.com/users/jnioche/orgs","repos_url":"https://api.github.com/users/jnioche/repos","events_url":"https://api.github.com/users/jnioche/events{/privacy}","received_events_url":"https://api.github.com/users/jnioche/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/DigitalPebble/storm-crawler/labels/bug","name":"bug","color":"fc2929"},{"url":"https://api.github.com/repos/DigitalPebble/storm-crawler/labels/fetcher","name":"fetcher","color":"bfe5bf"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":6,"created_at":"2015-02-06T00:15:16Z","updated_at":"2015-02-06T19:16:43Z","closed_at":null,"body":"I've just found that when looking at the code with Pradan, [ProtocolFactory](https://github.com/DigitalPebble/storm-crawler/blob/master/core/src/main/java/com/digitalpebble/storm/crawler/protocol/ProtocolFactory.java) caches the Protocol instances so there is exactly one instance within a ProtocolFactory. \r\n\r\nIn the FetcherBolt we have a single instance of ProtocolFactory, which is used by all the threads concurrently. The method  getProtocolOutput in the HTTP [protocol](https://github.com/DigitalPebble/storm-crawler/blob/master/core/src/main/java/com/digitalpebble/storm/crawler/protocol/http/HttpProtocol.java#L205) could be called concurrently. Any shared state like `ifModifiedSince` could then become corrupted.\r\n\r\nPradhan will write a test class to illustrate the issue. We could sync the method but that would create a bottleneck. A simpler approach would be for the ProtocolFactory to create a new instance of the protocol for every request, it might be inexpensive to do so."},"comment":{"url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/comments/73295615","html_url":"https://github.com/DigitalPebble/storm-crawler/issues/89#issuecomment-73295615","issue_url":"https://api.github.com/repos/DigitalPebble/storm-crawler/issues/89","id":73295615,"user":{"login":"jakekdodd","id":6845693,"avatar_url":"https://avatars.githubusercontent.com/u/6845693?v=3","gravatar_id":"","url":"https://api.github.com/users/jakekdodd","html_url":"https://github.com/jakekdodd","followers_url":"https://api.github.com/users/jakekdodd/followers","following_url":"https://api.github.com/users/jakekdodd/following{/other_user}","gists_url":"https://api.github.com/users/jakekdodd/gists{/gist_id}","starred_url":"https://api.github.com/users/jakekdodd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jakekdodd/subscriptions","organizations_url":"https://api.github.com/users/jakekdodd/orgs","repos_url":"https://api.github.com/users/jakekdodd/repos","events_url":"https://api.github.com/users/jakekdodd/events{/privacy}","received_events_url":"https://api.github.com/users/jakekdodd/received_events","type":"User","site_admin":false},"created_at":"2015-02-06T19:16:43Z","updated_at":"2015-02-06T19:16:43Z","body":"Yes absolutely!\n\n> On Feb 6, 2015, at 11:03, Pradhan Bagur Umesh <notifications@github.com> wrote:\n> \n> Jake, I am writing a test case for this. If its fine with you I can sneak\n> in the fix along with that.\n> \n> On Fri, Feb 6, 2015 at 10:40 AM, Jake Dodd <notifications@github.com> wrote:\n> \n> > Yep no problem, will be able to tackle it this evening\n> >\n> > > On Feb 6, 2015, at 07:29, Julien Nioche <notifications@github.com>\n> > wrote:\n> > >\n> > > +1 to passing the metadata. do you have time to fix this?\n> > >\n> > > —\n> > > Reply to this email directly or view it on GitHub.\n> > >\n> >\n> > —\n> > Reply to this email directly or view it on GitHub\n> > <https://github.com/DigitalPebble/storm-crawler/issues/89#issuecomment-73289412>\n> > .\n> >\n> —\n> Reply to this email directly or view it on GitHub.\n> \n"}}