Closed Bug 905405 Opened 9 years ago Closed 9 years ago

Data race on nsAppShell::mLastNativeEventScheduled

Categories

(Core :: Widget: Win32, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: bent.mozilla, Assigned: roc)

Details

Attachments

(1 file)

Many threads write this value, here's the main thread:

  xul.dll!ScheduleNativeEventCallback - nsappshell.cpp:167
  xul.dll!ProcessNextNativeEvent - nsappshell.cpp:243
  xul.dll!DoProcessNextNativeEvent - nsbaseappshell.cpp:139
  xul.dll!OnProcessNextEvent - nsbaseappshell.cpp:286
  xul.dll!ProcessNextEvent - nsthread.cpp:595
  xul.dll!NS_ProcessNextEvent - nsthreadutils.cpp:238
  xul.dll!Run - messagepump.cpp:81

And here's the timer thread (though any thread that calls Dispatch on the main thread will end up here I think):

  xul.dll!ScheduleNativeEventCallback - nsappshell.cpp:167
  xul.dll!OnDispatchedEvent - nsbaseappshell.cpp:235
  xul.dll!PutEvent - nsthread.cpp:362
  xul.dll!Dispatch - nsthread.cpp:404
  xul.dll!PostTimerEvent - nstimerimpl.cpp:670
  xul.dll!Run - timerthread.cpp:232

TimeStamp is > 128 bits on windows so there's no way this assignment is atomic. If we read at a bad time here http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsAppShell.cpp#241 then I'm not sure if we're going to mess up our reference count somehow.
Attached patch fixSplinter Review
I don't think this matters all that much. The check in ProcessNextNativeEvent is really only a heuristic.
Assignee: nobody → roc
Attachment #795270 - Flags: review?(benjamin)
Attachment #795270 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/1f52d046c2e7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.