Menu

#41 Remove internal existingVfs boolean in ZipEpgClient; fix bad close() implementation

0.20140530.0
verified
None
bug
9
7
zip-client
2014-12-14
2014-09-08
No

So there seems to be scenarios in ZipEpgClient.close() where the internal counter does not reach zero. That itself is the real underlying bug, but good luck trying to trace that one down so let's meet in the middle and add a new close(boolean force) to the EpgClient interface. When called with true, it will force a close regardless of internal state. For ZipEpgClient, this means ensure the zip fs file is closed, consequences be damned. Call this version in the finalize() method and anywhere else it seems appropriate. Should eliminate the AccessDeniedException errors I see every once in awhile in users' log files.

Discussion

  • Derek Battams

    Derek Battams - 2014-09-08

    Actually, looking at the code again, I think most of these problems stem from the internal existingVfs boolean. Based on that flag, within the context of the Sage process, close() will never close the fs after the initial object is created. I think I get away with this because of the daily service restarts I do for Colossus, but most (many anyways) users won't do a daily restart and so will definitely hit this scenario where close() never actually tries to close the file because this existingVfs boolean will always be set to true.

    Assuming I'm right, I actually feel pretty good about this in the sense that the code is rather stable if most people aren't hitting this defect on a daily basis.

    The fix: Remove that flag, but still handle the possibility of needing to load from an existing fs call (lines 146-152). But just remove that boolean and trust the internal count and assume no external callers to the cache file except those triggered by the plugin. With that assumption, we should be good. Assuming this is right, the new close(boolean) shouldn't be needed, which would be ideal -- instead just set the internal count to zero inside finalize() before calling close from there (and log if it's not already 0 b/c that should never happen).

     
  • Derek Battams

    Derek Battams - 2014-09-08
    • summary: Add new close(boolean force) to EpgClient interface; implement it in ZipEpgClient --> Remove internal existingVfs boolean in ZipEpgClient; fix bad close() implementation
     
  • Derek Battams

    Derek Battams - 2014-11-02
    • status: accepted --> fixed
    • Milestone: 0.20131021.3 --> 0.20140530.0
     
  • Derek Battams

    Derek Battams - 2014-11-02

    Fixed in api 20140530

     
  • Derek Battams

    Derek Battams - 2014-12-14
    • status: fixed --> verified
     

Log in to post a comment.