Bug 4470 - Word Selection failures when computer uptime exceeds 2^32 ms
Summary: Word Selection failures when computer uptime exceeds 2^32 ms
Status: RESOLVED FIXED
Alias: None
Product: Xamarin Studio
Classification: Desktop
Component: Text Editor ()
Version: 2.8.6
Hardware: Macintosh Mac OS
: Low normal
Target Milestone: ---
Assignee: Mike Krüger
URL:
: 6060 ()
Depends on:
Blocks:
 
Reported: 2012-04-16 15:31 UTC by Andy Korth
Modified: 2012-10-25 03:36 UTC (History)
4 users (show)

Tags: gtk
Is this bug a regression?: ---
Last known good build:


Attachments
Patch that should fix the issue (438 bytes, patch)
2012-07-19 13:23 UTC, Mikayla Hutchinson [MSFT]
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Andy Korth 2012-04-16 15:31:28 UTC
I've found several issues with text editing in monodevelop once my machine's uptime exceeds 2^32 ms. 

(2^32) milliseconds = 49.71 days

kortham@Rivest ~$ uptime
14:20  up 50 days, 14:03, 7 users, load averages: 2.95 3.07 3.40

Most notably, double clicking to select a word will not select a word, and instead selects normally as if I was just dragging over text. Restarting monodevelop doesn't help with this issue. 

I've also run into problems selecting text in the first 5 or so lines of my source files. This is a bit more inconsistent, although the general trend is that it seems to ignore clicks and mouse input to the text editing area, especially clicks other than the first click after focusing in the text editing window (assuming my focus was in the document outline or some other pane..)  The issue occurs whether code folding is off or not.. 

I understand that replication is difficult, although I've observed this three times since I began to suspect the problem was uptime related. I usually end up restarting the machine to fix it. I was suspicious when logging out and logging back in didn't fix the issue. 

I have replicated the issue in non-unity builds of Monodevelop, specifically 2.8.6.5.

I'm running OS X 10.7.3. I'm not sure if the issue occurs on other platforms.

There wasn't any helpful information that I saw in my console or logs.
Comment 1 Mike Krüger 2012-04-16 15:47:53 UTC
I think it's a gtk issue - but I've no clue what could cause that.
For sure I didn't implement planned obsolescence in the editor :)
Comment 2 Andy Korth 2012-04-16 16:19:40 UTC
Thanks for checking on the report Mike! I had poked around the code a bit last time- I was trying to look for cases where the absolute time was held in an int, which did might be the case in 

core/Mono.Texteditor/Mono.TextEditor/TextEditor.cs

protected override bool OnButtonPressEvent (Gdk.EventButton e)

And the NativeEventButtonStruct uses uints for time.. not sure if GDK

Although, yes, it all comes down to the e.Time, and I guess that's out of Gdk. I got lost trying to follow it back, perhaps there's nothing to be done. I understand if it's a low priority issue... I have system updates to do anyway.. hehe.
Comment 3 Mike Krüger 2012-04-16 16:26:46 UTC
low prio != unimportant :)

low prio (for me means) it's very uncommon, no data loss and probably not fixable :/
Comment 4 Mike Krüger 2012-04-17 01:12:02 UTC
hm, that would fail if the event time would stop when reaching uint.MaxValue ...
Comment 5 Mikayla Hutchinson [MSFT] 2012-04-26 19:37:35 UTC
Not sure how the double to guint32 conversion is meant to go, but this code looks mighty suspicious:
http://git.gnome.org/browse/gtk+/tree/gdk/quartz/gdkevents-quartz.c?h=gtk-2-24#n224
Comment 6 Mikayla Hutchinson [MSFT] 2012-04-26 19:47:58 UTC
Okay, Jeff confirmed that the cast does not wrap when it overflows. So the following change should fix it:

//make sure the value wraps when it overflows
return (guint32) (guint64) (time * 1000.0);

Mitch, does this look correct to you?
Comment 7 Michael Natterer 2012-04-27 03:48:20 UTC
I don't see how casting from one larger-than-32-bit type to another, and
then casting to 32 bit should change anything. I did a small test program:

  gdouble t = (gdouble) ((1 << 31)) + 1.0;

  g_print ("cast to guint32: %lu	cast via guint64: %lu\n",
           (guint32) (t * 1000.0),
           (guint32) (guint64) (t * 1000.0));

which prints:

cast to guint32: 1000	cast via guint64: 1000

So both ways correctly wrap around it seems.
Comment 8 Michael Natterer 2012-04-27 04:34:00 UTC
More for the use case, i get:

foo1: 1000	foo2: 1000

from:

/* gcc `pkg-config --cflags --libs glib-2.0` -o timeoverflow timeoverflow.c */

#include <glib.h>

static gdouble t = (gdouble) ((1 << 31)) + 1.0;

static guint32
foo1 (void)
{
  return t * 1000.0;
}

static guint32
foo2 (void)
{
  return (guint32) (guint64) (t * 1000.0);
}

gint
main (gint argc, gint *argv[])
{
  gdouble t = (gdouble) ((1 << 31)) + 1.0;

  g_print ("foo1: %lu	foo2: %lu\n", foo1(), foo2());

  return 1;
}
Comment 9 Jeffrey Stedfast 2012-04-27 16:11:52 UTC
This is the test case mhutch and I used to see if things would wrap properly:


#include <stdio.h>
#include <limits.h>

int main (int argc, char **argv)
{
	double d = (double) UINT_MAX + 2.0;
	unsigned long long ull = (unsigned long long) d;
	unsigned int u1 = (unsigned int) d;
	unsigned int u2 = (unsigned int) ull;

	printf ("double = %f; uint #1 = %u, uint #2 = %u\n", d, u1, u2);

	return 0;
}



[fejj@localhost ~]$ ./double
double = 4294967297.000000; uint #1 = 4294967295, uint #2 = 1
Comment 10 Jeffrey Stedfast 2012-04-27 16:31:23 UTC
aha, the behavior changes depending on the optimization level...

[fejj@localhost ~]$ gcc -m32 -O2 -o double double.c
[fejj@localhost ~]$ ./double
double = 4294967297.000000; uint #1 = 4294967295, uint #2 = 1


vs


[fejj@localhost ~]$ gcc -m32 -O1 -o double double.c
[fejj@localhost ~]$ ./double
double = 4294967297.000000; uint #1 = 4294967295, uint #2 = 1


vs


[fejj@localhost ~]$ gcc -m32 -O0 -o double double.c
[fejj@localhost ~]$ ./double
double = 4294967297.000000; uint #1 = 0, uint #2 = 1
Comment 11 Michael Natterer 2012-04-27 16:36:28 UTC
Argh, how evil, I suggest we put the double into a guint64 then
and explicitly modulo G_MAXUINT.

Not testing now because it's late and the mac is off,
do you agree this would work?
Comment 12 Mikayla Hutchinson [MSFT] 2012-04-27 16:50:41 UTC
It should, though I don't think it's necessary, uint64 to uint32 seems to wrap reliably, it's just the double to integer conversion that misbehaves.
Comment 13 Mikayla Hutchinson [MSFT] 2012-04-27 16:57:09 UTC
I guess that it worked on 64-bit because the double to integer conversion could go via the native int size then be cast down. I wouldn't be surprised if double to uint64 had similar wrapping issues at UINT64_MAX, but I don't think anyone will have uptime of half a billion years; -)
Comment 14 Mikayla Hutchinson [MSFT] 2012-07-10 14:07:43 UTC
Mitch, could we get this patched and upstreamed?
Comment 15 Mikayla Hutchinson [MSFT] 2012-07-10 14:08:56 UTC
*** Bug 6060 has been marked as a duplicate of this bug. ***
Comment 16 Michael Natterer 2012-07-10 14:20:48 UTC
Sure, could you remind me which of above options is the definite fix
again? :) I read the whole bug and I'm still confused, did anyone
try my suggestion in comment 11? If not, I'll try it and come up with
a patch for upstream.
Comment 17 Mikayla Hutchinson [MSFT] 2012-07-10 14:28:22 UTC
I didn't try your suggestion - according to Jeff's test in comment 9+10, the casting fix should work, which is simpler and cheaper.

uint #1 is the (guint32), and either zeroes or maxes depending on optimization level.
uint #2 is the (guint32) (guint64), and wraps at all optimization levels, which is what we need.

So my suggestion in comment #6 should work, though I haven't tested it.
Comment 18 Mikayla Hutchinson [MSFT] 2012-07-19 13:23:52 UTC
Created attachment 2233 [details]
Patch that should fix the issue

Patch attached. It builds, will try to confirm that it fixes the issue if/when my machine exceeds 49.7 days of uptime...
Comment 19 Mikayla Hutchinson [MSFT] 2012-07-19 13:31:38 UTC
Patch is now in bockbuild.
Comment 20 Michael Natterer 2012-08-23 03:21:03 UTC
Seems I forgot to upstream this over GUADEC, will take care of it right
away.
Comment 21 Michael Natterer 2012-08-23 03:33:34 UTC
Patch pushed to upstream master and gtk-2-24:

commit c0c3085128b0af739c73db31f9330508aad8f2e6
Author: Michael Natterer <mitch@gimp.org>
Date:   Thu Aug 23 09:28:13 2012 +0200

    quartz: add evil casting to make sure time wraps correctly on 32bit machines
    
    get_time_from_ns_event(): apply patch from Michael Hutchinson which
    makes sure the returned guint32 wraps correctly on 32 bit machines
    when the uptime exceeds 2^32 ms.
    (cherry picked from commit 78506bd604099161819ffdd0fdef98967f8980de)

 gdk/quartz/gdkevents-quartz.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Comment 22 Mike Krüger 2012-10-25 03:36:48 UTC
closing - should be fixed by a newer gtk build.