> http://cr.opensolaris.org/~dduvall/pkg-server-query/
This change looks fine to me. > - I've taken the subprocess module, ripped out its Windows half and > turned Popen into Mopen, which forks off and runs a method, rather than > forking and execing. There's some weirdness here I'm not entirely sure > about, but it's been working pretty well for me so far. What kind of weirdness are you seeing? > - That's all used to allow the server to (re)generate the search database > asynchronously, so we don't have to wait several minutes before the > server starts, erm, serving. So there's a bunch of code to make sure > it does something sane while the search service is unavailable. I was a bit concerned about this code at first, but I think I've convinced myself that there isn't a problem. You're grabbing a lock in a signal handler, which can cause a deadlock if you take a signal on top of the owner of the lock. It doesn't look like that can happen in this case. (Do you know how, or in what context, the signal handlers get executed in Python?) I'm also assuming that the catalog isn't multithreaded, even though you've imported the threading module. (Just for the lock, correct?) I was also concerned that you had a possibility for a race with the searchdb_update_handle. That variable gets updated without any synchronization. I'm also pretty sure that there's no problem here; although, I was initially worried that an update to searchdb from transaction could deadlock with a deferred update from the signal handler. Assuming that we're not multi-threaded and that the signal handler can't be preempted, I think everything is okay. Would update the comment at catalog:101 to include exactly which portions of the catalog state are protected by the lock? It would be cool if we could extend Lock to contain the owner of the lock so that we could implement a Python version of MUTEX_HELD(). Then in update_index we could assert LOCK_HELD for its callers. We can file an RFE for this and do it later, though. -j _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
