[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
I'm sorry for the late response, I've been quite busy last week. 2009/4/27 Amanda Walker ama...@chromium.org: 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 sto...@chromium.org: 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 ama...@chromium.org: 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: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe:
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
Thanks for elaborating. So if we look at it another way, there are several different problems that ProcessSingleton is currently used to solve: - Prevent multiple instances of Chromium from referring to the same profile/user data directory. In the general sense, what we need here is a way to acquire and test locks on data directories. This is a cross-platform requirement: all platforms should be able to launch instances of Chromium in different user sessions with different user data / profiles (in order to support fast user switching on Mac Windows, single Linux boxes with multiple people logged in, etc. Currently, ProcessSingleton really only solves this by accident. It seems cleaner to me to meet this requirement with an explicit mechanism: Something like a ProfileLock or DirectoryLock, for example. This also gives a way to ease this requirement if someone fixes the concurrent-access problem, without affecting other uses of ProcessSingleton. - Testing, since a number of tests are not multiple-instance safe. I would describe this as a defect in those tests or the test framework, which again ProcessSingleton only mitigates by accident. I would prefer we address this by fixing the tests and/or test scripts so that we can indeed run tests in parallel without worrying about them stepping on each other. We have a strong desire to reduce our build cycle time, of which having to run tests serially instead of in parallel is a large component. - Within a user session, ensuring that requests to open URLs get processed by a currently running instance of Chromium instead of launching a new one. This is where platform differences are strongest, and is ProcessSingleton's main function. Registering properties on windows or the like for Windows and X11 seems cleaner to me than putting breadcrumbs into the file system, since window properties are inherently tied to the user session and to an active instance of the application (and thus do not need to be cleaned up on a crash, etc.). On the Mac, this function is provided by the OS, so there's no need for Chromium to duplicate it. --Amanda On Tue, May 5, 2009 at 12:27 PM, Никита Офицеров himi...@gmail.com wrote: I'm sorry for the late response, I've been quite busy last week. 2009/4/27 Amanda Walker ama...@chromium.org: 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 sto...@chromium.org: 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
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
I agree with the earlier argument about not larding startup with things like writing new files to id the coming-up Chrome to late-coming instances. An alternative might be to acquire a lock to protect the profile, and write an id asynchronously after startup. The late-coming instance would see the lock and wait for the id to appear (or the lock to evaporate). This does mean the late-coming instance will have to wait sometimes, but in those cases the coming-up instance presumably is not ready to respond to requests anyhow. On Mac and Linux, all we need for the lock is an agreed-upon open file descriptor, which should be easy to find. We could acquire the lock in parallel with other startup activity which doesn't modify system state. -scott On Tue, May 5, 2009 at 10:16 AM, Amanda Walker ama...@chromium.org wrote: Thanks for elaborating. So if we look at it another way, there are several different problems that ProcessSingleton is currently used to solve: - Prevent multiple instances of Chromium from referring to the same profile/user data directory. In the general sense, what we need here is a way to acquire and test locks on data directories. This is a cross-platform requirement: all platforms should be able to launch instances of Chromium in different user sessions with different user data / profiles (in order to support fast user switching on Mac Windows, single Linux boxes with multiple people logged in, etc. Currently, ProcessSingleton really only solves this by accident. It seems cleaner to me to meet this requirement with an explicit mechanism: Something like a ProfileLock or DirectoryLock, for example. This also gives a way to ease this requirement if someone fixes the concurrent-access problem, without affecting other uses of ProcessSingleton. - Testing, since a number of tests are not multiple-instance safe. I would describe this as a defect in those tests or the test framework, which again ProcessSingleton only mitigates by accident. I would prefer we address this by fixing the tests and/or test scripts so that we can indeed run tests in parallel without worrying about them stepping on each other. We have a strong desire to reduce our build cycle time, of which having to run tests serially instead of in parallel is a large component. - Within a user session, ensuring that requests to open URLs get processed by a currently running instance of Chromium instead of launching a new one. This is where platform differences are strongest, and is ProcessSingleton's main function. Registering properties on windows or the like for Windows and X11 seems cleaner to me than putting breadcrumbs into the file system, since window properties are inherently tied to the user session and to an active instance of the application (and thus do not need to be cleaned up on a crash, etc.). On the Mac, this function is provided by the OS, so there's no need for Chromium to duplicate it. --Amanda On Tue, May 5, 2009 at 12:27 PM, Никита Офицеров himi...@gmail.com wrote: I'm sorry for the late response, I've been quite busy last week. 2009/4/27 Amanda Walker ama...@chromium.org: 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 sto...@chromium.org: 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
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On May 5, 10:44 am, Scott Hess sh...@chromium.org wrote: I agree with the earlier argument about not larding startup with things like writing new files to id the coming-up Chrome to late-coming instances. An alternative might be to acquire a lock to protect the profile, and write an id asynchronously after startup. The late-coming instance would see the lock and wait for the id to appear (or the lock to evaporate). This does mean the late-coming instance will have to wait sometimes, but in those cases the coming-up instance presumably is not ready to respond to requests anyhow. On Mac and Linux, all we need for the lock is an agreed-upon open file descriptor, which should be easy to find. We could acquire the lock in parallel with other startup activity which doesn't modify system state. -scott On Tue, May 5, 2009 at 10:16 AM, Amanda Walker ama...@chromium.org wrote: Thanks for elaborating. So if we look at it another way, there are several different problems that ProcessSingleton is currently used to solve: - Prevent multiple instances of Chromium from referring to the same profile/user data directory. In the general sense, what we need here is a way to acquire and test locks on data directories. This is a cross-platform requirement: all platforms should be able to launch instances of Chromium in different user sessions with different user data / profiles (in order to support fast user switching on Mac Windows, single Linux boxes with multiple people logged in, etc. Currently, ProcessSingleton really only solves this by accident. It seems cleaner to me to meet this requirement with an explicit mechanism: Something like a ProfileLock or DirectoryLock, for example. This also gives a way to ease this requirement if someone fixes the concurrent-access problem, without affecting other uses of ProcessSingleton. - Testing, since a number of tests are not multiple-instance safe. I would describe this as a defect in those tests or the test framework, which again ProcessSingleton only mitigates by accident. I would prefer we address this by fixing the tests and/or test scripts so that we can indeed run tests in parallel without worrying about them stepping on each other. We have a strong desire to reduce our build cycle time, of which having to run tests serially instead of in parallel is a large component. - Within a user session, ensuring that requests to open URLs get processed by a currently running instance of Chromium instead of launching a new one. This is where platform differences are strongest, and is ProcessSingleton's main function. Registering properties on windows or the like for Windows and X11 seems cleaner to me than putting breadcrumbs into the file system, since window properties are inherently tied to the user session and to an active instance of the application (and thus do not need to be cleaned up on a crash, etc.). On the Mac, this function is provided by the OS, so there's no need for Chromium to duplicate it. --Amanda On Tue, May 5, 2009 at 12:27 PM, Никита Офицеров himi...@gmail.com wrote: I'm sorry for the late response, I've been quite busy last week. 2009/4/27 Amanda Walker ama...@chromium.org: 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 sto...@chromium.org: 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. I know of no browser that would create tabs/windows across sessions backed by the same process. Come to think of it I know of no product that does this. Maybe some service that interacts with the user. Hence quite a lot of changes are required for this to function, like delegating handling of a widget to the newly
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Sun, Apr 26, 2009 at 5:50 PM, Amanda Walker ama...@chromium.org wrote: Application startup is one of the areas where we count every millisecond, and try to touch the disk as little as possible. I don't think it's safe to assume that the cost of creating and writing to a file is negligible in this context without actually measuring it. Just curious, how many files are read/written loading a profile: history, bookmarks, cache, last session, preferences, cookies, etc? I imagine it's non-trivial and happens at every startup. I agree that the proposal just for the sake of consolidating code may not be warranted, but to jump all over it because a file has to be written seems like premature optimization in light of everything else that happens on the disk at app startup. -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
Many of those are done lazily specifically so that they won't slow down app launch. However, I didn't mean to come across as jumping all over the idea--I was just saying we shouldn't brush it off as insignificant without measuring it. --Amanda On Mon, Apr 27, 2009 at 11:05 AM, Mike Pinkerton pinker...@chromium.org wrote: On Sun, Apr 26, 2009 at 5:50 PM, Amanda Walker ama...@chromium.org wrote: Application startup is one of the areas where we count every millisecond, and try to touch the disk as little as possible. I don't think it's safe to assume that the cost of creating and writing to a file is negligible in this context without actually measuring it. Just curious, how many files are read/written loading a profile: history, bookmarks, cache, last session, preferences, cookies, etc? I imagine it's non-trivial and happens at every startup. I agree that the proposal just for the sake of consolidating code may not be warranted, but to jump all over it because a file has to be written seems like premature optimization in light of everything else that happens on the disk at app startup. -- Mike Pinkerton Mac Weenie pinker...@google.com --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Mon, Apr 27, 2009 at 11:21 AM, Evan Martin e...@chromium.org wrote: That leaves process_util for Linux and Mac, which uses fuser (or something similar) and that is a hack. However, it's only used for ui tests, I believe -- it's not needed in normal usage. The Mac uses ps -xw, but the same reasoning applies. It's rather hacky, but I'm in the same position as you, that since it's only used during testing, that's OK. (And in fact, the Mac version as it exists today would only work during testing so we're sure that no one will start using it in the actual app.) I am not opposed to (lazily, on a background thread) writing a pid file on Linux, though I'd prefer some other solution if one could be found. I'm not fond of the idea, as it just litters things up. And since it's only needed for testing the app, but it would be written all the time (testing or not) it's just wasteful. Maybe it's to just remove the need for ChromeBrowserProcessId() somehow. It would be nice, but that's used to test the spawning of processes, so it might not be easily expurgated. Avi --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Mon, Apr 27, 2009 at 8:05 AM, Mike Pinkerton pinker...@chromium.org wrote: On Sun, Apr 26, 2009 at 5:50 PM, Amanda Walker ama...@chromium.org wrote: Application startup is one of the areas where we count every millisecond, and try to touch the disk as little as possible. I don't think it's safe to assume that the cost of creating and writing to a file is negligible in this context without actually measuring it. Just curious, how many files are read/written loading a profile: history, bookmarks, cache, last session, preferences, cookies, etc? I imagine it's non-trivial and happens at every startup. Cache is loaded synchronously on the I/O thread, along with the cookies. The history is loaded asynchronously on the history thread. The bookmarks are loaded asynchronously on the file thread and passed to the UI thread. The only things loaded synchronously are the ones we have to have to continue, which are the preferences and, when open the last session is set in the preferences, the session file. I agree that the proposal just for the sake of consolidating code may not be warranted, but to jump all over it because a file has to be written seems like premature optimization in light of everything else that happens on the disk at app startup. We start up significantly faster than any other browsers because we worry about this stuff. When the disk is thrashing (like right after you start your computer) there will be an even larger difference. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Thu, Apr 23, 2009 at 4:35 PM, Nikita Ofitserov himi...@gmail.com wrote: Currently there are different implementations of ProcessSingleton and ChromeBrowserProcessId() on Windows, Linux and Mac. Most of them are quite hacky, so there should be a better way. I think current IPC system with slight modifications can replace them all. 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. This eliminates all differences between platforms in process_singleton_* and chrome_process_util_*. What do you think? But it also has additional costs compared to the current implementations: new race conditions, more error handling required to clean up stale files, more disk I/O at application startup, etc. Is there a significant corresponding benefit? 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. --Amanda --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On 25 апр, 05:09, Peter Kasting pkast...@chromium.org wrote: On Thu, Apr 23, 2009 at 1:35 PM, Nikita Ofitserov himi...@gmail.com wrote: I suggest: Don't try to get pid dynamically, but create on startup in datadir file 'ChromePid' or something like that with pid. Creating files during startup would slow startup too much. Chrome crashing would leave a stale PID file lying around, confusing future launches. PK We need to create this file only once, during main process startup, not for every renderer/plugin process. Sorry if I haven't made it clear. I don't think that creating a text file once with one number would slow startup too much. And we don't really care about stale PID file. If future launch will find it there, it will search for process with that pid, then it will try to connect to IPC channel - if either of this fails, it will just continue launch, rewriting the file. So no confusion here. Nikita Ofitserov --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Suggestion for crossplatform ProcessSingleton and ChromeBrowserProcessId()
On Thu, Apr 23, 2009 at 1:35 PM, Nikita Ofitserov himi...@gmail.com wrote: I suggest: Don't try to get pid dynamically, but create on startup in datadir file 'ChromePid' or something like that with pid. Creating files during startup would slow startup too much. Chrome crashing would leave a stale PID file lying around, confusing future launches. PK --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
