On 29 Jun 2016, at 02:06, David T. Lewis <lewis@mail.msen.com> wrote:

On Tue, Jun 28, 2016 at 04:47:00PM -0500, Chris Muller wrote:

...

On Tue, Jun 28, 2016 at 5:04 AM, Max Leske <maxleske@gmail.com> wrote:

...

Hi,

Opening the same image twice works fine as long as no writes to the .changes file occur. When both images write to the .changes file however it will be broken for both because the offsets for the changes are wrong. This can lead to lost data and predominantly to invalid method source code, which is a pain with Monticello.

I suggest that we implement a kind of lock mechanism to ensure that only one image (the first one opened) can write to the .changes file.

If the offsets are wrong in this scenario, it's a bug in the image. The image is supposed to seek to the end of the changes file before writing the next chunk. While this sounds horrible in theory, in practice it works remarkably well, and I have been happily surprised at how reliable it is after many years of using and abusing the feature. That is a very good thing.

Adding a lock to prevent the scenario would be bad, because it would surely break a number of other legitimate use cases.

...

...

I???ve opened an issue for Pharo here: https://pharo.fogbugz.com/f/cases/18635/The-changes-file-should-be-bound-to-...

I have several applications which launch multiple copies of the same image for multicore processing. The images do their work, commit it to database, then exit themselves without saving. Its a great feature.

Doing work is not the problem. Modifying source code is the problem.

That is consistent with my experience. I remember expecting horrible things to happen if I had two images sharing a changes file, but nothing bad ever happened. It just works.

...

I know OSProcess, when combined with CommandShell, has a RemoteTask which allows efficient forking of the image (via Linux copy-on-write memory sharing) and so a solution like what happens in Windows is not really good.

My assumption with RemoteTask was that someone doing complex or long-running jobs would more or less know what they were doing, and would have the good sense to stop writing to the changes file from a bunch of forked images. But in actual practice, I have never seen a problem related to this. It just works.

...

Instead of putting a pop-up in front of the user, perhaps one way to solve the problem would be to, upon image save, simply goes through all the changes since the last save and re-flushes them to the .changes file.

That way, if someone does want to save the same image on top of themself, at least it would be whichever saved last "wins"....

There must be a problem somewhere, otherwise Max would not be raising the issue. So whatever combination of operating system and image is having a problem, I would be inclined fix that.

:) Thanks Dave!

Windows cannot be a problem, because the operating system will not permit you to open the changes file twice. The Unix/Linux systems that I have used all work fine.

Max, which operating system/VM/image are you using? Is this on a Mac?

Mac OS X 10.11.5, Pharo 6 (60086)

Dave

I actually didn’t open the issue for myself but because of a student who ran into this. I’ve been in the same situation before but I’m an experienced user while students at the research group sometimes just spend a couple of weeks with Pharo and then such things are a real problem. Interestingly the issue is, as you already suggested, pretty hard to reproduce i.e., modifying arbitrary methods in both images did not show the symptoms I was looking for. Here’s a reproducible case (at least on my machine): 1. create a new method in both images: foo ^ nil 2. Modify it in one image: foo "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum." ^ nil + 1 3. Modify it in the other image: foo ^ nil - 1 isEmpty ifTrue: [ "blah" nil ] In my case saving in step three produces a syntax error when the source is loaded from file again. I don’t really have a clue as to what the underlying issue is, but I suspect it may have to do with comments and a particular situation in which the position is not being correctly updated before or after writing. I agree with Chris that locks may be problematic, it just seemed like the simplest obvious solution (although of course it gets complicated when an image crashes and doesn’t clean up the lock…). Cheers, Max

On 29 Jun 2016, at 14:45, David T. Lewis <lewis@mail.msen.com> wrote:

Max,

Confirming on Linux and Squeak. See below.

On Wed, Jun 29, 2016 at 08:53:12AM +0200, Max Leske wrote:

...

...

On 29 Jun 2016, at 02:06, David T. Lewis <lewis@mail.msen.com> wrote:

On Tue, Jun 28, 2016 at 04:47:00PM -0500, Chris Muller wrote:

...

On Tue, Jun 28, 2016 at 5:04 AM, Max Leske <maxleske@gmail.com> wrote:

...

Hi,

Opening the same image twice works fine as long as no writes to the .changes file occur. When both images write to the .changes file however it will be broken for both because the offsets for the changes are wrong. This can lead to lost data and predominantly to invalid method source code, which is a pain with Monticello.

I suggest that we implement a kind of lock mechanism to ensure that only one image (the first one opened) can write to the .changes file.

If the offsets are wrong in this scenario, it's a bug in the image. The image is supposed to seek to the end of the changes file before writing the next chunk. While this sounds horrible in theory, in practice it works remarkably well, and I have been happily surprised at how reliable it is after many years of using and abusing the feature. That is a very good thing.

Adding a lock to prevent the scenario would be bad, because it would surely break a number of other legitimate use cases.

...

...

I???ve opened an issue for Pharo here: https://pharo.fogbugz.com/f/cases/18635/The-changes-file-should-be-bound-to-...

I have several applications which launch multiple copies of the same image for multicore processing. The images do their work, commit it to database, then exit themselves without saving. Its a great feature.

Doing work is not the problem. Modifying source code is the problem.

...

That is consistent with my experience. I remember expecting horrible things to happen if I had two images sharing a changes file, but nothing bad ever happened. It just works.

...

I know OSProcess, when combined with CommandShell, has a RemoteTask which allows efficient forking of the image (via Linux copy-on-write memory sharing) and so a solution like what happens in Windows is not really good.

My assumption with RemoteTask was that someone doing complex or long-running jobs would more or less know what they were doing, and would have the good sense to stop writing to the changes file from a bunch of forked images. But in actual practice, I have never seen a problem related to this. It just works.

...

Instead of putting a pop-up in front of the user, perhaps one way to solve the problem would be to, upon image save, simply goes through all the changes since the last save and re-flushes them to the .changes file.

That way, if someone does want to save the same image on top of themself, at least it would be whichever saved last "wins"....

There must be a problem somewhere, otherwise Max would not be raising the issue. So whatever combination of operating system and image is having a problem, I would be inclined fix that.

:) Thanks Dave!

...

Windows cannot be a problem, because the operating system will not permit you to open the changes file twice. The Unix/Linux systems that I have used all work fine.

Max, which operating system/VM/image are you using? Is this on a Mac?

Mac OS X 10.11.5, Pharo 6 (60086)

...

Dave

I actually didn???t open the issue for myself but because of a student who ran into this. I???ve been in the same situation before but I???m an experienced user while students at the research group sometimes just spend a couple of weeks with Pharo and then such things are a real problem.

Interestingly the issue is, as you already suggested, pretty hard to reproduce i.e., modifying arbitrary methods in both images did not show the symptoms I was looking for.

Here???s a reproducible case (at least on my machine):

1. create a new method in both images:

foo ^ nil

2. Modify it in one image:

foo "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum." ^ nil + 1

3. Modify it in the other image:

foo ^ nil - 1 isEmpty ifTrue: [ "blah" nil ]

Confirmed on Linux + Squeak.

I did your test above using #forkSqueak so that I had two identical images sharing the same changes file. In each image, I saved the #foo method. At that point, the changes file conntained exactly what I would expect.

I then did a save and exit from the child image, followed by a save and exit from the original image. I can see that the changes from the child image are now overwriting the changes from the original parent image. Since the parent image is the one that was saved last, its #foo method now has corrupted source.

This is not a scenario that I have ever encountered, but I can see how it might happen in a classroom setting.

I can't look into this further right now, but it seems possible that the problem happens only when saving the image, in which case we could force the changes file to seek to end of file before doing the save. But we'll need to do some more testing to make sure that this is the only scenario in which it happens.

Dave

Great, thanks. In the scenario I described I did not save either image. Of course, without saving the problem will not exist as soon as you start the image anew (the old pointers are still valid and new content will be written to the end). The problem does exhibit itself without saving though. Since this is not anything critical, don’t put too much effort into it. I’ll have time in a couple of weeks to look at it in detail and then, once we understand the problem, we can discuss possible solutions. Cheers, Max

...

In my case saving in step three produces a syntax error when the source is loaded from file again. I don???t really have a clue as to what the underlying issue is, but I suspect it may have to do with comments and a particular situation in which the position is not being correctly updated before or after writing.

I agree with Chris that locks may be problematic, it just seemed like the simplest obvious solution (although of course it gets complicated when an image crashes and doesn???t clean up the lock???).

Cheers, Max

On 30 Jun 2016, at 05:09, Ben Coman <btc@openInWorld.com> wrote:

On Tue, Jun 28, 2016 at 6:04 PM, Max Leske <maxleske@gmail.com> wrote:

...

Hi,

Opening the same image twice works fine as long as no writes to the .changes file occur. When both images write to the .changes file however it will be broken for both because the offsets for the changes are wrong. This can lead to lost data and predominantly to invalid method source code, which is a pain with Monticello.

I suggest that we implement a kind of lock mechanism to ensure that only one image (the first one opened) can write to the .changes file.

I’ve opened an issue for Pharo here: https://pharo.fogbugz.com/f/cases/18635/The-changes-file-should-be-bound-to-...

Cheers, Max

I just learnt something quite surprising that is probably important to be aware of... "Locks given by fcntl are not associated with the file-descriptor or open-file table entries. Instead, they are bound to the process itself. For example, a process has multiple open file descriptors for a particular file and gets a read/write lock using any one of these descriptors. Now closing any of these file descriptors will release the lock, the process holds on the file. The descriptor that was used to acquire the lock in the first place might still be open, but the process will loose its lock. So, it does not require an explicit unlock or a close ONLY on the descriptor that was used to acquire the lock in fcntl call. Doing unlock or close on any of the open file descriptors will release the lock owned by the process on the particular file."

https://loonytek.com/2015/01/15/advisory-file-locking-differences-between-po...

cheers -ben

Which would solve the problem of a crashed image not cleaning up its lock. Thanks for sharing Ben.