I'm sorry for the late response, I've been quite busy last week. 2009/4/27 Amanda Walker <[email protected]>: > "Hacky" is fairly subjective: are there particular things about the > existing implementation that bother you? Currently they are different > for each OS because each OS has its own issues surrounding launching > multiple instances of the browser, different mechanisms for handling > incoming requests from the OS to open web pages, and so on. >
2009/4/28 stoyan <[email protected]>: > Well, on Windows code looks racy, does not work well across multiple > desktops, and probably will create troubles accessing userdata when > same instance is running in different terminal sessions (with same > user account). The problem with quick, baroque source code is that usually there are lots of corner cases. I have rethought the idea after your comments, and there are several problems with current Windows implementation: 1. As Stoyan mentioned, window messaging does not work across multiple desktops. Therefore it doesn't work across multiple terminal sessions. Consider the following use case: A user has a version of Windows that allows multiple consequent terminal sessions on his PC, and leaves Chrome opened. Then he opens a session from his laptop and launches Chrome. Preferred result is opening a new window in the second session, but AFAIK it's impossible for the Chrome master process to open a window in a different window station. Hence quite a lot of changes are required for this to function, like delegating handling of a widget to the newly created process, and these fall out of the scope of this idea. But current ProcessSingleton will just fail to detect another Chrome instance and crash or show weird behavior. If it worked, we could at least show some message telling user that running Chrome from different terminal sessions is not yet implemented. So IMHO ProcessSingleton should not rely on window messages, that are by definition bound to one window station. 2. Another problem with Windows implementation (maybe not really a problem) is that it guards Chrome against being launched with the same (luckily, case-insensitive) user data dir path. This is a fundamental problem, because ProcessSingleton should protect not only against being launched with the same path, but against actually using the same dir twice (one directory can be addressed with different paths, because of links and mounting, even on Windows NTFS partitions). Well, I think that without user intervention a situation that exploits this cannot be created, so it may be considered as ok, but it's still a problem that IMHO should be fixed. Because of that, I suggest two ideas: 1. Do almost as I suggested before, but instead of Chrome PID use a GUID, and write it only once, on the first run. This GUID will let us distinguish data dirs without need to rely on their path. So at normal launch it will be one additional disk read (just several bytes) and some IPC. If you think that we can't afford even one extra disk read, then think about my second idea: 2. If problem #2 is not really a problem, then instead of GUID or PID we can just use some hash from user data dir path as channel_id, and make no disk operations at all. This is by no means worse than current implementation and solves the first problem. However, this is only a partial solution. As an additional benefit, ProcessSingleton becomes what it was meant to be - a facility to ensure that only one instance with given data dir is running. Opening new URLs and getting browser PID will be just IPC messages, without custom protocol parsing or some extra quirks in ProcessSingleton code. 2009/4/27 Amanda Walker <[email protected]>: > Simply eliminating differences between per-platform implementations is > not necessarily a large benefit by itself, since the amount of code is > small and concerns an area where the three platforms behave quite > differently. I do agree with this, but it's not only for eliminating differences between implementations, but also to improve them. There is already quite a lot of platform-specific code in IPC, and it can handle differences quite well. The only remaining thing is disabling this functionality completely on Mac when not running tests, and this can be handled by one 'if'. As for "racyness", this can also be improved. Current implementation was racy because singleton was created with some delay after checking that no Chrome process existed. That was a bug, and that was fixed by adding singleton creation in constructor. It isn't actually atomic, but should work well. Creating named pipe is atomic operation now (FILE_FLAG_FIRST_PIPE_INSTANCE), however, creating UNIX domain socket is not. So this may also be improved in order to avoid race conditions. Nikita Ofitserov --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
